public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v51 2/7] Add conditional lock feature to dshash 2+ messages / 2 participants [nested] [flat]
* [PATCH v51 2/7] Add conditional lock feature to dshash @ 2020-03-13 07:58 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 2+ 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(Wed_Mar_10_12_10_39_2021_432)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v51-0003-Make-archiver-process-an-auxiliary-process.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: effective_io_concurrency and NVMe devices @ 2022-04-21 08:14 David Rowley <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: David Rowley @ 2022-04-21 08:14 UTC (permalink / raw) To: Bruce Momjian <[email protected]>; +Cc: pgsql-hackers On Wed, 20 Apr 2022 at 14:56, Bruce Momjian <[email protected]> wrote: > NVMe devices have a maximum queue length of 64k: > Should we increase its maximum to 64k? Backpatched? (SATA has a > maximum queue length of 256.) I have a machine here with 1 x PCIe 3.0 NVMe SSD and also 1 x PCIe 4.0 NVMe SSD. I ran a few tests to see how different values of effective_io_concurrency would affect performance. I tried to come up with a query that did little enough CPU processing to ensure that I/O was the clear bottleneck. The test was with a 128GB table on a machine with 64GB of RAM. I padded the tuples out so there were 4 per page so that the aggregation didn't have much work to do. The query I ran was: explain (analyze, buffers, timing off) select count(p) from r where a = 1; Here's what I saw: NVME PCIe 3.0 (Samsung 970 Evo 1TB) e_i_c query_time_ms 0 88627.221 1 652915.192 5 271536.054 10 141168.986 100 67340.026 1000 70686.596 10000 70027.938 100000 70106.661 Saw a max of 991 MB/sec in iotop NVME PCIe 4.0 (Samsung 980 Pro 1TB) e_i_c query_time_ms 0 59306.960 1 956170.704 5 237879.121 10 135004.111 100 55662.030 1000 51513.717 10000 59807.824 100000 53443.291 Saw a max of 1126 MB/sec in iotop I'm not pretending that this is the best query and table size to show it, but at least this test shows that there's not much to gain by prefetching further. I imagine going further than we need to is likely to have negative consequences due to populating the kernel page cache with buffers that won't be used for a while. I also imagine going too far out likely increases the risk that buffers we've prefetched are evicted before they're used. This does also highlight that an effective_io_concurrency of 1 (the default) is pretty terrible in this test. The bitmap contained every 2nd page. I imagine that would break normal page prefetching by the kernel. If that's true, then it does not explain why e_i_c = 0 was so fast. I've attached the test setup that I did. I'm open to modifying the test and running again if someone has an idea that might show benefits to larger values for effective_io_concurrency. David Attachments: [application/octet-stream] setup.sql (1.1K, ../../CAApHDvpOjk-LPXvSbRcNGbuux8iw3JBwtjWDBNDkirwAC3S17g@mail.gmail.com/2-setup.sql) download ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2022-04-21 08:14 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-13 07:58 [PATCH v51 2/7] Add conditional lock feature to dshash Kyotaro Horiguchi <[email protected]> 2022-04-21 08:14 Re: effective_io_concurrency and NVMe devices David Rowley <[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