Re: scalability bottlenecks with (many) partitions (and more) - Mailing list pgsql-hackers

Tomas Vondra <tomas@vondra.me> writes:
> I've finally pushed this, after many rounds of careful testing to ensure
> no regressions, and polishing.

Coverity is not terribly happy with this.  "Assert(fpPtr = fpEndPtr);"
is very clearly not doing what you presumably intended.  The others
look like overaggressive assertion checking.  If you don't want those
macros to assume that the argument is unsigned, you could force the
issue, say with

 #define FAST_PATH_GROUP(index)    \
-    (AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_BACKEND)), \
+    (AssertMacro((uint32) (index) < FP_LOCK_SLOTS_PER_BACKEND), \
      ((index) / FP_LOCK_SLOTS_PER_GROUP))


________________________________________________________________________________________________________
*** CID 1619664:  Incorrect expression  (ASSERT_SIDE_EFFECT)
/srv/coverity/git/pgsql-git/postgresql/src/backend/storage/lmgr/proc.c: 325 in InitProcGlobal()
319             pg_atomic_init_u32(&(proc->procArrayGroupNext), INVALID_PROC_NUMBER);
320             pg_atomic_init_u32(&(proc->clogGroupNext), INVALID_PROC_NUMBER);
321             pg_atomic_init_u64(&(proc->waitStart), 0);
322         }
323
324         /* Should have consumed exactly the expected amount of fast-path memory. */
>>>     CID 1619664:  Incorrect expression  (ASSERT_SIDE_EFFECT)
>>>     Assignment "fpPtr = fpEndPtr" has a side effect.  This code will work differently in a non-debug build.
325         Assert(fpPtr = fpEndPtr);
326
327         /*
328          * Save pointers to the blocks of PGPROC structures reserved for auxiliary
329          * processes and prepared transactions.
330          */

________________________________________________________________________________________________________
*** CID 1619662:  Integer handling issues  (NO_EFFECT)
/srv/coverity/git/pgsql-git/postgresql/src/backend/storage/lmgr/lock.c: 3731 in GetLockStatusData()
3725
3726             LWLockAcquire(&proc->fpInfoLock, LW_SHARED);
3727
3728             for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; ++f)
3729             {
3730                 LockInstanceData *instance;
>>>     CID 1619662:  Integer handling issues  (NO_EFFECT)
>>>     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "f >= 0U".
3731                 uint32        lockbits = FAST_PATH_GET_BITS(proc, f);
3732
3733                 /* Skip unallocated slots. */
3734                 if (!lockbits)
3735                     continue;
3736

________________________________________________________________________________________________________
*** CID 1619661:  Integer handling issues  (NO_EFFECT)
/srv/coverity/git/pgsql-git/postgresql/src/backend/storage/lmgr/lock.c: 2696 in FastPathGrantRelationLock()
2690         uint32        group = FAST_PATH_REL_GROUP(relid);
2691
2692         /* Scan for existing entry for this relid, remembering empty slot. */
2693         for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
2694         {
2695             /* index into the whole per-backend array */
>>>     CID 1619661:  Integer handling issues  (NO_EFFECT)
>>>     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "group >= 0U".
2696             uint32        f = FAST_PATH_SLOT(group, i);
2697
2698             if (FAST_PATH_GET_BITS(MyProc, f) == 0)
2699                 unused_slot = f;
2700             else if (MyProc->fpRelId[f] == relid)
2701             {

________________________________________________________________________________________________________
*** CID 1619660:  Integer handling issues  (NO_EFFECT)
/srv/coverity/git/pgsql-git/postgresql/src/backend/storage/lmgr/lock.c: 2813 in FastPathTransferRelationLocks()
2807
2808             for (j = 0; j < FP_LOCK_SLOTS_PER_GROUP; j++)
2809             {
2810                 uint32        lockmode;
2811
2812                 /* index into the whole per-backend array */
>>>     CID 1619660:  Integer handling issues  (NO_EFFECT)
>>>     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "group >= 0U".
2813                 uint32        f = FAST_PATH_SLOT(group, j);
2814
2815                 /* Look for an allocated slot matching the given relid. */
2816                 if (relid != proc->fpRelId[f] || FAST_PATH_GET_BITS(proc, f) == 0)
2817                     continue;
2818

________________________________________________________________________________________________________
*** CID 1619659:  Integer handling issues  (NO_EFFECT)
/srv/coverity/git/pgsql-git/postgresql/src/backend/storage/lmgr/lock.c: 3067 in GetLockConflicts()
3061
3062                 for (j = 0; j < FP_LOCK_SLOTS_PER_GROUP; j++)
3063                 {
3064                     uint32        lockmask;
3065
3066                     /* index into the whole per-backend array */
>>>     CID 1619659:  Integer handling issues  (NO_EFFECT)
>>>     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "group >= 0U".
3067                     uint32        f = FAST_PATH_SLOT(group, j);
3068
3069                     /* Look for an allocated slot matching the given relid. */
3070                     if (relid != proc->fpRelId[f])
3071                         continue;
3072                     lockmask = FAST_PATH_GET_BITS(proc, f);

________________________________________________________________________________________________________
*** CID 1619658:  Integer handling issues  (NO_EFFECT)
/srv/coverity/git/pgsql-git/postgresql/src/backend/storage/lmgr/lock.c: 2739 in FastPathUnGrantRelationLock()
2733         uint32        group = FAST_PATH_REL_GROUP(relid);
2734
2735         FastPathLocalUseCounts[group] = 0;
2736         for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
2737         {
2738             /* index into the whole per-backend array */
>>>     CID 1619658:  Integer handling issues  (NO_EFFECT)
>>>     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "group >= 0U".
2739             uint32        f = FAST_PATH_SLOT(group, i);
2740
2741             if (MyProc->fpRelId[f] == relid
2742                 && FAST_PATH_CHECK_LOCKMODE(MyProc, f, lockmode))
2743             {
2744                 Assert(!result);

________________________________________________________________________________________________________
*** CID 1619657:  Integer handling issues  (NO_EFFECT)
/srv/coverity/git/pgsql-git/postgresql/src/backend/storage/lmgr/lock.c: 2878 in FastPathGetRelationLockEntry()
2872
2873         for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
2874         {
2875             uint32        lockmode;
2876
2877             /* index into the whole per-backend array */
>>>     CID 1619657:  Integer handling issues  (NO_EFFECT)
>>>     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "group >= 0U".
2878             uint32        f = FAST_PATH_SLOT(group, i);
2879
2880             /* Look for an allocated slot matching the given relid. */
2881             if (relid != MyProc->fpRelId[f] || FAST_PATH_GET_BITS(MyProc, f) == 0)
2882                 continue;
2883

            regards, tom lane