Re: scalability bottlenecks with (many) partitions (and more)
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Tomas Vondra <tomas@vondra.me>
Cc: Jakub Wartak <jakub.wartak@enterprisedb.com>,
Robert Haas <robertmhaas@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
Andres Freund <andres@anarazel.de>
Date: 2024-09-22T15:45:59Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Make FP_LOCK_SLOTS_PER_BACKEND look like a function
- c878de1db438 18.0 landed
-
Fix asserts in fast-path locking code
- a7e5237f268e 18.0 landed
-
Increase the number of fast-path lock slots
- c4d5cb71d229 18.0 landed
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