public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v49 2/7] Add conditional lock feature to dshash
7+ messages / 4 participants
[nested] [flat]
* [PATCH v49 2/7] Add conditional lock feature to dshash
@ 2020-03-13 07:58 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Kyotaro Horiguchi @ 2020-03-13 07:58 UTC (permalink / raw)
Dshash currently waits for lock unconditionally. It is inconvenient
when we want to avoid being blocked by other processes. This commit
adds alternative functions of dshash_find and dshash_find_or_insert
that allows immediate return on lock failure.
---
src/backend/lib/dshash.c | 98 +++++++++++++++++++++-------------------
src/include/lib/dshash.h | 3 ++
2 files changed, 55 insertions(+), 46 deletions(-)
diff --git a/src/backend/lib/dshash.c b/src/backend/lib/dshash.c
index 520bfa0979..853d78b528 100644
--- a/src/backend/lib/dshash.c
+++ b/src/backend/lib/dshash.c
@@ -383,6 +383,10 @@ dshash_get_hash_table_handle(dshash_table *hash_table)
* the caller must take care to ensure that the entry is not left corrupted.
* The lock mode is either shared or exclusive depending on 'exclusive'.
*
+ * If found is not NULL, *found is set to true if the key is found in the hash
+ * table. If the key is not found, *found is set to false and a pointer to a
+ * newly created entry is returned.
+ *
* The caller must not lock a lock already.
*
* Note that the lock held is in fact an LWLock, so interrupts will be held on
@@ -392,36 +396,7 @@ dshash_get_hash_table_handle(dshash_table *hash_table)
void *
dshash_find(dshash_table *hash_table, const void *key, bool exclusive)
{
- dshash_hash hash;
- size_t partition;
- dshash_table_item *item;
-
- hash = hash_key(hash_table, key);
- partition = PARTITION_FOR_HASH(hash);
-
- Assert(hash_table->control->magic == DSHASH_MAGIC);
- Assert(!hash_table->find_locked);
-
- LWLockAcquire(PARTITION_LOCK(hash_table, partition),
- exclusive ? LW_EXCLUSIVE : LW_SHARED);
- ensure_valid_bucket_pointers(hash_table);
-
- /* Search the active bucket. */
- item = find_in_bucket(hash_table, key, BUCKET_FOR_HASH(hash_table, hash));
-
- if (!item)
- {
- /* Not found. */
- LWLockRelease(PARTITION_LOCK(hash_table, partition));
- return NULL;
- }
- else
- {
- /* The caller will free the lock by calling dshash_release_lock. */
- hash_table->find_locked = true;
- hash_table->find_exclusively_locked = exclusive;
- return ENTRY_FROM_ITEM(item);
- }
+ return dshash_find_extended(hash_table, key, exclusive, false, false, NULL);
}
/*
@@ -439,31 +414,60 @@ dshash_find_or_insert(dshash_table *hash_table,
const void *key,
bool *found)
{
- dshash_hash hash;
- size_t partition_index;
- dshash_partition *partition;
+ return dshash_find_extended(hash_table, key, true, false, true, found);
+}
+
+
+/*
+ * Find the key in the hash table.
+ *
+ * "exclusive" is the lock mode in which the partition for the returned item
+ * is locked. If "nowait" is true, the function immediately returns if
+ * required lock was not acquired. "insert" indicates insert mode. In this
+ * mode new entry is inserted and set *found to false. *found is set to true if
+ * found. "found" must be non-null in this mode.
+ */
+void *
+dshash_find_extended(dshash_table *hash_table, const void *key,
+ bool exclusive, bool nowait, bool insert, bool *found)
+{
+ dshash_hash hash = hash_key(hash_table, key);
+ size_t partidx = PARTITION_FOR_HASH(hash);
+ dshash_partition *partition = &hash_table->control->partitions[partidx];
+ LWLockMode lockmode = exclusive ? LW_EXCLUSIVE : LW_SHARED;
dshash_table_item *item;
- hash = hash_key(hash_table, key);
- partition_index = PARTITION_FOR_HASH(hash);
- partition = &hash_table->control->partitions[partition_index];
-
- Assert(hash_table->control->magic == DSHASH_MAGIC);
- Assert(!hash_table->find_locked);
+ /* must be exclusive when insert allowed */
+ Assert(!insert || (exclusive && found != NULL));
restart:
- LWLockAcquire(PARTITION_LOCK(hash_table, partition_index),
- LW_EXCLUSIVE);
+ if (!nowait)
+ LWLockAcquire(PARTITION_LOCK(hash_table, partidx), lockmode);
+ else if (!LWLockConditionalAcquire(PARTITION_LOCK(hash_table, partidx),
+ lockmode))
+ return NULL;
+
ensure_valid_bucket_pointers(hash_table);
/* Search the active bucket. */
item = find_in_bucket(hash_table, key, BUCKET_FOR_HASH(hash_table, hash));
if (item)
- *found = true;
+ {
+ if (found)
+ *found = true;
+ }
else
{
- *found = false;
+ if (found)
+ *found = false;
+
+ if (!insert)
+ {
+ /* The caller didn't told to add a new entry. */
+ LWLockRelease(PARTITION_LOCK(hash_table, partidx));
+ return NULL;
+ }
/* Check if we are getting too full. */
if (partition->count > MAX_COUNT_PER_PARTITION(hash_table))
@@ -479,7 +483,8 @@ restart:
* Give up our existing lock first, because resizing needs to
* reacquire all the locks in the right order to avoid deadlocks.
*/
- LWLockRelease(PARTITION_LOCK(hash_table, partition_index));
+ LWLockRelease(PARTITION_LOCK(hash_table, partidx));
+
resize(hash_table, hash_table->size_log2 + 1);
goto restart;
@@ -493,12 +498,13 @@ restart:
++partition->count;
}
- /* The caller must release the lock with dshash_release_lock. */
+ /* The caller will free the lock by calling dshash_release_lock. */
hash_table->find_locked = true;
- hash_table->find_exclusively_locked = true;
+ hash_table->find_exclusively_locked = exclusive;
return ENTRY_FROM_ITEM(item);
}
+
/*
* Remove an entry by key. Returns true if the key was found and the
* corresponding entry was removed.
diff --git a/src/include/lib/dshash.h b/src/include/lib/dshash.h
index a6ea377173..5b8114d041 100644
--- a/src/include/lib/dshash.h
+++ b/src/include/lib/dshash.h
@@ -91,6 +91,9 @@ extern void *dshash_find(dshash_table *hash_table,
const void *key, bool exclusive);
extern void *dshash_find_or_insert(dshash_table *hash_table,
const void *key, bool *found);
+extern void *dshash_find_extended(dshash_table *hash_table, const void *key,
+ bool exclusive, bool nowait, bool insert,
+ bool *found);
extern bool dshash_delete_key(dshash_table *hash_table, const void *key);
extern void dshash_delete_entry(dshash_table *hash_table, void *entry);
extern void dshash_release_lock(dshash_table *hash_table, void *entry);
--
2.27.0
----Next_Part(Tue_Mar__9_16_53_11_2021_575)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v49-0003-Make-archiver-process-an-auxiliary-process.patch"
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Increase NUM_XLOGINSERT_LOCKS
@ 2025-01-17 08:28 Yura Sokolov <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Yura Sokolov @ 2025-01-17 08:28 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: [email protected] <[email protected]>; Zhou, Zhiguo <[email protected]>; wenhui qiu <[email protected]>
Excuse me, Andres, I've found I've pressed wrong button when I sent this
letter first time, and it was sent only to you. So I'm sending the copy now.
Please, reply to this message with copy of your answer. Your answer is
really valuable to be published in the list.
16.01.2025 18:36, Andres Freund wrote:
> Hi,
>
> On 2025-01-16 16:52:46 +0300, Yura Sokolov wrote:
>> Good day, hackers.
>>
>> Zhiguo Zhow proposed to transform xlog reservation to lock-free
algorighm to
>> increment NUM_XLOGINSERT_LOCKS on very huge (480vCPU) servers. [1]
>>
>> While I believe lock-free reservation make sense on huge server, it
is hard
>> to measure on small servers and personal computers/notebooks.
>>
>> But increase of NUM_XLOGINSERT_LOCKS have measurable performance
gain (using
>> synthetic test) even on my working notebook:
>>
>> Ryzen-5825U (8 cores, 16 threads) limited to 2GHz , Ubuntu 24.04
>
> I've experimented with this in the past.
>
>
> Unfortunately increasing it substantially can make the contention on the
> spinlock *substantially* worse.
>
> c=80 && psql -c checkpoint -c 'select pg_switch_wal()' && pgbench -n
-M prepared -c$c -j$c -f <(echo "SELECT pg_logical_emit_message(true,
'test', repeat('0', 1024*1024));";) -P1 -T15
>
> On a 2x Xeon Gold 5215, with max_wal_size = 150GB and the workload
ran a few
> times to ensure WAL is already allocated.
>
> With
> NUM_XLOGINSERT_LOCKS = 8: 1459 tps
> NUM_XLOGINSERT_LOCKS = 80: 2163 tps
So, even in your test you have +50% gain from increasing
NUM_XLOGINSERT_LOCKS.
(And that is why I'm keen on smaller increase, like upto 64, not 128).
>
> The main reason is that the increase in insert locks puts a lot more
pressure
> on the spinlock.
That it addressed by Zhiguo Zhow and me in other thread [1]. But
increasing NUM_XLOGINSERT_LOCKS gives benefits right now (at least on
smaller installations), and "lock-free reservation" should be measured
against it.
> Secondarily it's also that we spend more time iterating
> through the insert locks when waiting, and that that causes a lot of
cacheline
> pingpong.
Waiting is done with LWLockWaitForVar, and there is no wait if
`insertingAt` is in future. It looks very efficient in master branch code.
> On much larger machines this gets considerably worse. IIRC I saw
something
> like an 8x regression on a large machine in the past, but I couldn't
find the
> actual numbers anymore, so I wouldn't want to bet on it.
I believe, it should be remeasured.
[1]
https://postgr.es/m/flat/PH7PR11MB5796659F654F9BE983F3AD97EF142%40PH7PR11MB5796.namprd11.prod.outloo...
------
regards
Yura
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Increase NUM_XLOGINSERT_LOCKS
@ 2025-01-18 11:53 Yura Sokolov <[email protected]>
parent: Yura Sokolov <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Yura Sokolov @ 2025-01-18 11:53 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: [email protected] <[email protected]>; Zhou, Zhiguo <[email protected]>; wenhui qiu <[email protected]>
Since it seems Andres missed my request to send answer's copy,
here it is:
On 2025-01-16 18:55:47 +0300, Yura Sokolov wrote:
> 16.01.2025 18:36, Andres Freund пишет:
>> Hi,
>>
>> On 2025-01-16 16:52:46 +0300, Yura Sokolov wrote:
>>> Good day, hackers.
>>>
>>> Zhiguo Zhow proposed to transform xlog reservation to lock-free
algorighm to
>>> increment NUM_XLOGINSERT_LOCKS on very huge (480vCPU) servers. [1]
>>>
>>> While I believe lock-free reservation make sense on huge server, it
is hard
>>> to measure on small servers and personal computers/notebooks.
>>>
>>> But increase of NUM_XLOGINSERT_LOCKS have measurable performance
gain (using
>>> synthetic test) even on my working notebook:
>>>
>>> Ryzen-5825U (8 cores, 16 threads) limited to 2GHz , Ubuntu 24.04
>>
>> I've experimented with this in the past.
>>
>>
>> Unfortunately increasing it substantially can make the contention on the
>> spinlock *substantially* worse.
>>
>> c=80 && psql -c checkpoint -c 'select pg_switch_wal()' && pgbench -n
-M prepared -c$c -j$c -f <(echo "SELECT pg_logical_emit_message(true,
'test', repeat('0', 1024*1024));";) -P1 -T15
>>
>> On a 2x Xeon Gold 5215, with max_wal_size = 150GB and the workload
ran a few
>> times to ensure WAL is already allocated.
>>
>> With
>> NUM_XLOGINSERT_LOCKS = 8: 1459 tps
>> NUM_XLOGINSERT_LOCKS = 80: 2163 tps
>
> So, even in your test you have +50% gain from increasing
> NUM_XLOGINSERT_LOCKS.
>
> (And that is why I'm keen on smaller increase, like upto 64, not 128).
Oops, I swapped the results around when reformatting the results, sorry!
It's
the opposite way. I.e. increasing the locks hurts.
Here's that issue fixed and a few more NUM_XLOGINSERT_LOCKS. This is a
slightly different disk (the other seems to have to go the way of the dodo),
so the results aren't expected to be exactly the same.
NUM_XLOGINSERT_LOCKS TPS
1 2583
2 2524
4 2711
8 2788
16 1938
32 1834
64 1865
128 1543
>>
>> The main reason is that the increase in insert locks puts a lot more
pressure
>> on the spinlock.
>
> That it addressed by Zhiguo Zhow and me in other thread [1]. But
increasing
> NUM_XLOGINSERT_LOCKS gives benefits right now (at least on smaller
> installations), and "lock-free reservation" should be measured
against it.
I know that there's that thread, I just don't see how we can increase
NUM_XLOGINSERT_LOCKS due to the regressions it can cause.
>> Secondarily it's also that we spend more time iterating
>> through the insert locks when waiting, and that that causes a lot of
cacheline
>> pingpong.
>
> Waiting is done with LWLockWaitForVar, and there is no wait if
`insertingAt`
> is in future. It looks very efficient in master branch code.
But LWLockWaitForVar is called from WaitXLogInsertionsToFinish, which just
iterates over all locks.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Increase NUM_XLOGINSERT_LOCKS
@ 2025-01-23 02:30 Japin Li <[email protected]>
parent: Yura Sokolov <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Japin Li @ 2025-01-23 02:30 UTC (permalink / raw)
To: Yura Sokolov <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected] <[email protected]>; Zhou, Zhiguo <[email protected]>; wenhui qiu <[email protected]>
On Sat, 18 Jan 2025 at 14:53, Yura Sokolov <[email protected]> wrote:
> Since it seems Andres missed my request to send answer's copy,
> here it is:
>
> On 2025-01-16 18:55:47 +0300, Yura Sokolov wrote:
>> 16.01.2025 18:36, Andres Freund пишет:
>>> Hi,
>>>
>>> On 2025-01-16 16:52:46 +0300, Yura Sokolov wrote:
>>>> Good day, hackers.
>>>>
>>>> Zhiguo Zhow proposed to transform xlog reservation to lock-free
> algorighm to
>>>> increment NUM_XLOGINSERT_LOCKS on very huge (480vCPU) servers. [1]
>>>>
>>>> While I believe lock-free reservation make sense on huge server,
> it is hard
>>>> to measure on small servers and personal computers/notebooks.
>>>>
>>>> But increase of NUM_XLOGINSERT_LOCKS have measurable performance
> gain (using
>>>> synthetic test) even on my working notebook:
>>>>
>>>> Ryzen-5825U (8 cores, 16 threads) limited to 2GHz , Ubuntu 24.04
>>>
>>> I've experimented with this in the past.
>>>
>>>
>>> Unfortunately increasing it substantially can make the contention on the
>>> spinlock *substantially* worse.
>>>
>>> c=80 && psql -c checkpoint -c 'select pg_switch_wal()' && pgbench
> -n -M prepared -c$c -j$c -f <(echo "SELECT
> pg_logical_emit_message(true, 'test', repeat('0', 1024*1024));";)
> -P1 -T15
>>>
>>> On a 2x Xeon Gold 5215, with max_wal_size = 150GB and the workload
> ran a few
>>> times to ensure WAL is already allocated.
>>>
>>> With
>>> NUM_XLOGINSERT_LOCKS = 8: 1459 tps
>>> NUM_XLOGINSERT_LOCKS = 80: 2163 tps
>>
>> So, even in your test you have +50% gain from increasing
>> NUM_XLOGINSERT_LOCKS.
>>
>> (And that is why I'm keen on smaller increase, like upto 64, not 128).
>
> Oops, I swapped the results around when reformatting the results,
> sorry! It's
> the opposite way. I.e. increasing the locks hurts.
>
> Here's that issue fixed and a few more NUM_XLOGINSERT_LOCKS. This is a
> slightly different disk (the other seems to have to go the way of the dodo),
> so the results aren't expected to be exactly the same.
>
> NUM_XLOGINSERT_LOCKS TPS
> 1 2583
> 2 2524
> 4 2711
> 8 2788
> 16 1938
> 32 1834
> 64 1865
> 128 1543
>
>
>>>
>>> The main reason is that the increase in insert locks puts a lot
> more pressure
>>> on the spinlock.
>>
>> That it addressed by Zhiguo Zhow and me in other thread [1]. But
> increasing
>> NUM_XLOGINSERT_LOCKS gives benefits right now (at least on smaller
>> installations), and "lock-free reservation" should be measured
> against it.
>
> I know that there's that thread, I just don't see how we can increase
> NUM_XLOGINSERT_LOCKS due to the regressions it can cause.
>
>
>>> Secondarily it's also that we spend more time iterating
>>> through the insert locks when waiting, and that that causes a lot
> of cacheline
>>> pingpong.
>>
>> Waiting is done with LWLockWaitForVar, and there is no wait if
> `insertingAt`
>> is in future. It looks very efficient in master branch code.
>
> But LWLockWaitForVar is called from WaitXLogInsertionsToFinish, which just
> iterates over all locks.
>
Hi, Yura Sokolov
I tested the patch on Hygon C86 7490 64-core using benchmarksql 5.0 with
500 warehouses and 256 terminals run time 10 mins:
| case | min | avg | max |
|--------------------+--------------+--------------+--------------|
| master (4108440) | 891,225.77 | 904,868.75 | 913,708.17 |
| lock 64 | 1,007,716.95 | 1,012,013.22 | 1,018,674.00 |
| lock 64 attempt 1 | 1,016,716.07 | 1,017,735.55 | 1,019,328.36 |
| lock 64 attempt 2 | 1,015,328.31 | 1,018,147.74 | 1,021,513.14 |
| lock 128 | 1,010,147.38 | 1,014,128.11 | 1,018,672.01 |
| lock 128 attempt 1 | 1,018,154.79 | 1,023,348.35 | 1,031,365.42 |
| lock 128 attempt 2 | 1,013,245.56 | 1,018,984.78 | 1,023,696.00 |
I didn't NUM_XLOGINSERT_LOCKS with 16 and 32, however, I tested it with 256,
and got the following error:
2025-01-23 02:23:23.828 CST [333524] PANIC: too many LWLocks taken
I hope this test will be helpful.
--
Regrads,
Japin Li
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Increase NUM_XLOGINSERT_LOCKS
@ 2025-01-23 05:41 wenhui qiu <[email protected]>
parent: Japin Li <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: wenhui qiu @ 2025-01-23 05:41 UTC (permalink / raw)
To: Japin Li <[email protected]>; +Cc: Yura Sokolov <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; Zhou, Zhiguo <[email protected]>
HI Japin
Thank you for you test ,It seems NUM_XLOGINSERT_LOCKS 64 is great , I
think it doesn't need to grow much,What do you think?
Regards
On Thu, Jan 23, 2025 at 10:30 AM Japin Li <[email protected]> wrote:
> On Sat, 18 Jan 2025 at 14:53, Yura Sokolov <[email protected]>
> wrote:
> > Since it seems Andres missed my request to send answer's copy,
> > here it is:
> >
> > On 2025-01-16 18:55:47 +0300, Yura Sokolov wrote:
> >> 16.01.2025 18:36, Andres Freund пишет:
> >>> Hi,
> >>>
> >>> On 2025-01-16 16:52:46 +0300, Yura Sokolov wrote:
> >>>> Good day, hackers.
> >>>>
> >>>> Zhiguo Zhow proposed to transform xlog reservation to lock-free
> > algorighm to
> >>>> increment NUM_XLOGINSERT_LOCKS on very huge (480vCPU) servers. [1]
> >>>>
> >>>> While I believe lock-free reservation make sense on huge server,
> > it is hard
> >>>> to measure on small servers and personal computers/notebooks.
> >>>>
> >>>> But increase of NUM_XLOGINSERT_LOCKS have measurable performance
> > gain (using
> >>>> synthetic test) even on my working notebook:
> >>>>
> >>>> Ryzen-5825U (8 cores, 16 threads) limited to 2GHz , Ubuntu 24.04
> >>>
> >>> I've experimented with this in the past.
> >>>
> >>>
> >>> Unfortunately increasing it substantially can make the contention on
> the
> >>> spinlock *substantially* worse.
> >>>
> >>> c=80 && psql -c checkpoint -c 'select pg_switch_wal()' && pgbench
> > -n -M prepared -c$c -j$c -f <(echo "SELECT
> > pg_logical_emit_message(true, 'test', repeat('0', 1024*1024));";)
> > -P1 -T15
> >>>
> >>> On a 2x Xeon Gold 5215, with max_wal_size = 150GB and the workload
> > ran a few
> >>> times to ensure WAL is already allocated.
> >>>
> >>> With
> >>> NUM_XLOGINSERT_LOCKS = 8: 1459 tps
> >>> NUM_XLOGINSERT_LOCKS = 80: 2163 tps
> >>
> >> So, even in your test you have +50% gain from increasing
> >> NUM_XLOGINSERT_LOCKS.
> >>
> >> (And that is why I'm keen on smaller increase, like upto 64, not 128).
> >
> > Oops, I swapped the results around when reformatting the results,
> > sorry! It's
> > the opposite way. I.e. increasing the locks hurts.
> >
> > Here's that issue fixed and a few more NUM_XLOGINSERT_LOCKS. This is a
> > slightly different disk (the other seems to have to go the way of the
> dodo),
> > so the results aren't expected to be exactly the same.
> >
> > NUM_XLOGINSERT_LOCKS TPS
> > 1 2583
> > 2 2524
> > 4 2711
> > 8 2788
> > 16 1938
> > 32 1834
> > 64 1865
> > 128 1543
> >
> >
> >>>
> >>> The main reason is that the increase in insert locks puts a lot
> > more pressure
> >>> on the spinlock.
> >>
> >> That it addressed by Zhiguo Zhow and me in other thread [1]. But
> > increasing
> >> NUM_XLOGINSERT_LOCKS gives benefits right now (at least on smaller
> >> installations), and "lock-free reservation" should be measured
> > against it.
> >
> > I know that there's that thread, I just don't see how we can increase
> > NUM_XLOGINSERT_LOCKS due to the regressions it can cause.
> >
> >
> >>> Secondarily it's also that we spend more time iterating
> >>> through the insert locks when waiting, and that that causes a lot
> > of cacheline
> >>> pingpong.
> >>
> >> Waiting is done with LWLockWaitForVar, and there is no wait if
> > `insertingAt`
> >> is in future. It looks very efficient in master branch code.
> >
> > But LWLockWaitForVar is called from WaitXLogInsertionsToFinish, which
> just
> > iterates over all locks.
> >
>
> Hi, Yura Sokolov
>
> I tested the patch on Hygon C86 7490 64-core using benchmarksql 5.0 with
> 500 warehouses and 256 terminals run time 10 mins:
>
> | case | min | avg | max |
> |--------------------+--------------+--------------+--------------|
> | master (4108440) | 891,225.77 | 904,868.75 | 913,708.17 |
> | lock 64 | 1,007,716.95 | 1,012,013.22 | 1,018,674.00 |
> | lock 64 attempt 1 | 1,016,716.07 | 1,017,735.55 | 1,019,328.36 |
> | lock 64 attempt 2 | 1,015,328.31 | 1,018,147.74 | 1,021,513.14 |
> | lock 128 | 1,010,147.38 | 1,014,128.11 | 1,018,672.01 |
> | lock 128 attempt 1 | 1,018,154.79 | 1,023,348.35 | 1,031,365.42 |
> | lock 128 attempt 2 | 1,013,245.56 | 1,018,984.78 | 1,023,696.00 |
>
> I didn't NUM_XLOGINSERT_LOCKS with 16 and 32, however, I tested it with
> 256,
> and got the following error:
>
> 2025-01-23 02:23:23.828 CST [333524] PANIC: too many LWLocks taken
>
> I hope this test will be helpful.
>
> --
> Regrads,
> Japin Li
>
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Increase NUM_XLOGINSERT_LOCKS
@ 2025-01-23 12:50 Yura Sokolov <[email protected]>
parent: wenhui qiu <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Yura Sokolov @ 2025-01-23 12:50 UTC (permalink / raw)
To: wenhui qiu <[email protected]>; Japin Li <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected] <[email protected]>; Zhou, Zhiguo <[email protected]>
23.01.2025 08:41, wenhui qiu wrote:
> HI Japin
> Thank you for you test ,It seems NUM_XLOGINSERT_LOCKS 64
> is great , I think it doesn't need to grow much,What do you think?
I agree: while 128 shows small benefit, it is not as big at the moment.
Given there's other waiting issues (may) arise from increasing it, 64
seems to be sweet spot.
Probably in a future it could be increased more after other places will
be optimized.
> On Thu, Jan 23, 2025 at 10:30 AM Japin Li <[email protected]
> <mailto:[email protected]>> wrote:
>
> On Sat, 18 Jan 2025 at 14:53, Yura Sokolov <[email protected]
> <mailto:[email protected]>> wrote:
> > Since it seems Andres missed my request to send answer's copy,
> > here it is:
> >
> > On 2025-01-16 18:55:47 +0300, Yura Sokolov wrote:
> >> 16.01.2025 18:36, Andres Freund пишет:
> >>> Hi,
> >>>
> >>> On 2025-01-16 16:52:46 +0300, Yura Sokolov wrote:
> >>>> Good day, hackers.
> >>>>
> >>>> Zhiguo Zhow proposed to transform xlog reservation to lock-free
> > algorighm to
> >>>> increment NUM_XLOGINSERT_LOCKS on very huge (480vCPU) servers. [1]
> >>>>
> >>>> While I believe lock-free reservation make sense on huge server,
> > it is hard
> >>>> to measure on small servers and personal computers/notebooks.
> >>>>
> >>>> But increase of NUM_XLOGINSERT_LOCKS have measurable performance
> > gain (using
> >>>> synthetic test) even on my working notebook:
> >>>>
> >>>> Ryzen-5825U (8 cores, 16 threads) limited to 2GHz , Ubuntu
> 24.04
> >>>
> >>> I've experimented with this in the past.
> >>>
> >>>
> >>> Unfortunately increasing it substantially can make the
> contention on the
> >>> spinlock *substantially* worse.
> >>>
> >>> c=80 && psql -c checkpoint -c 'select pg_switch_wal()' && pgbench
> > -n -M prepared -c$c -j$c -f <(echo "SELECT
> > pg_logical_emit_message(true, 'test', repeat('0', 1024*1024));";)
> > -P1 -T15
> >>>
> >>> On a 2x Xeon Gold 5215, with max_wal_size = 150GB and the workload
> > ran a few
> >>> times to ensure WAL is already allocated.
> >>>
> >>> With
> >>> NUM_XLOGINSERT_LOCKS = 8: 1459 tps
> >>> NUM_XLOGINSERT_LOCKS = 80: 2163 tps
> >>
> >> So, even in your test you have +50% gain from increasing
> >> NUM_XLOGINSERT_LOCKS.
> >>
> >> (And that is why I'm keen on smaller increase, like upto 64, not
> 128).
> >
> > Oops, I swapped the results around when reformatting the results,
> > sorry! It's
> > the opposite way. I.e. increasing the locks hurts.
> >
> > Here's that issue fixed and a few more NUM_XLOGINSERT_LOCKS.
> This is a
> > slightly different disk (the other seems to have to go the way of
> the dodo),
> > so the results aren't expected to be exactly the same.
> >
> > NUM_XLOGINSERT_LOCKS TPS
> > 1 2583
> > 2 2524
> > 4 2711
> > 8 2788
> > 16 1938
> > 32 1834
> > 64 1865
> > 128 1543
> >
> >
> >>>
> >>> The main reason is that the increase in insert locks puts a lot
> > more pressure
> >>> on the spinlock.
> >>
> >> That it addressed by Zhiguo Zhow and me in other thread [1]. But
> > increasing
> >> NUM_XLOGINSERT_LOCKS gives benefits right now (at least on smaller
> >> installations), and "lock-free reservation" should be measured
> > against it.
> >
> > I know that there's that thread, I just don't see how we can increase
> > NUM_XLOGINSERT_LOCKS due to the regressions it can cause.
> >
> >
> >>> Secondarily it's also that we spend more time iterating
> >>> through the insert locks when waiting, and that that causes a lot
> > of cacheline
> >>> pingpong.
> >>
> >> Waiting is done with LWLockWaitForVar, and there is no wait if
> > `insertingAt`
> >> is in future. It looks very efficient in master branch code.
> >
> > But LWLockWaitForVar is called from WaitXLogInsertionsToFinish,
> which just
> > iterates over all locks.
> >
>
> Hi, Yura Sokolov
>
> I tested the patch on Hygon C86 7490 64-core using benchmarksql 5.0 with
> 500 warehouses and 256 terminals run time 10 mins:
>
> | case | min | avg | max |
> |--------------------+--------------+--------------+--------------|
> | master (4108440) | 891,225.77 | 904,868.75 | 913,708.17 |
> | lock 64 | 1,007,716.95 | 1,012,013.22 | 1,018,674.00 |
> | lock 64 attempt 1 | 1,016,716.07 | 1,017,735.55 | 1,019,328.36 |
> | lock 64 attempt 2 | 1,015,328.31 | 1,018,147.74 | 1,021,513.14 |
> | lock 128 | 1,010,147.38 | 1,014,128.11 | 1,018,672.01 |
> | lock 128 attempt 1 | 1,018,154.79 | 1,023,348.35 | 1,031,365.42 |
> | lock 128 attempt 2 | 1,013,245.56 | 1,018,984.78 | 1,023,696.00 |
>
> I didn't NUM_XLOGINSERT_LOCKS with 16 and 32, however, I tested it
> with 256,
> and got the following error:
>
> 2025-01-23 02:23:23.828 CST [333524] PANIC: too many LWLocks taken
>
> I hope this test will be helpful.
>
> --
> Regrads,
> Japin Li
>
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Increase NUM_XLOGINSERT_LOCKS
@ 2025-01-23 13:45 Japin Li <[email protected]>
parent: Yura Sokolov <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Japin Li @ 2025-01-23 13:45 UTC (permalink / raw)
To: Yura Sokolov <[email protected]>; +Cc: wenhui qiu <[email protected]>; Andres Freund <[email protected]>; [email protected] <[email protected]>; Zhou, Zhiguo <[email protected]>
On Thu, 23 Jan 2025 at 15:50, Yura Sokolov <[email protected]> wrote:
> 23.01.2025 08:41, wenhui qiu wrote:
>> HI Japin
>> Thank you for you test ,It seems NUM_XLOGINSERT_LOCKS 64
>> is great , I think it doesn't need to grow much,What do you think?
>
> I agree: while 128 shows small benefit, it is not as big at the moment.
> Given there's other waiting issues (may) arise from increasing it, 64
> seems to be sweet spot.
>
> Probably in a future it could be increased more after other places
> will be optimized.
>
+1.
--
Regrads,
Japin Li
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2025-01-23 13:45 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-13 07:58 [PATCH v49 2/7] Add conditional lock feature to dshash Kyotaro Horiguchi <[email protected]>
2025-01-17 08:28 Re: Increase NUM_XLOGINSERT_LOCKS Yura Sokolov <[email protected]>
2025-01-18 11:53 ` Re: Increase NUM_XLOGINSERT_LOCKS Yura Sokolov <[email protected]>
2025-01-23 02:30 ` Re: Increase NUM_XLOGINSERT_LOCKS Japin Li <[email protected]>
2025-01-23 05:41 ` Re: Increase NUM_XLOGINSERT_LOCKS wenhui qiu <[email protected]>
2025-01-23 12:50 ` Re: Increase NUM_XLOGINSERT_LOCKS Yura Sokolov <[email protected]>
2025-01-23 13:45 ` Re: Increase NUM_XLOGINSERT_LOCKS Japin Li <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox