public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v46 2/7] Add conditional lock feature to dshash 3+ messages / 3 participants [nested] [flat]
* [PATCH v46 2/7] Add conditional lock feature to dshash @ 2020-03-13 07:58 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 3+ 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(Thu_Jan_14_15_14_25_2021_903)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v46-0003-Make-archiver-process-an-auxiliary-process.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* pg_waldump vs. all-zeros WAL files; server creation of such files @ 2023-08-13 03:15 Noah Misch <[email protected]> 2023-08-14 01:37 ` Re: pg_waldump vs. all-zeros WAL files; server creation of such files Michael Paquier <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Noah Misch @ 2023-08-13 03:15 UTC (permalink / raw) To: pgsql-hackers The attached 010_zero.pl, when run as part of the pg_waldump test suite, fails at today's master (c36b636) and v15 (1bc19df). It passes at v14 (5a32af3). Command "pg_waldump --start 0/01000000 --end 0/01000100" fails as follows: pg_waldump: error: WAL segment size must be a power of two between 1 MB and 1 GB, but the WAL file "000000010000000000000002" header specifies 0 bytes Where it fails, the server has created an all-zeros WAL file under that name. Where it succeeds, that file doesn't exist at all. Two decisions to make: - Should a clean server shutdown ever leave an all-zeros WAL file? I think yes, it's okay to let that happen. - Should "pg_waldump --start $X --end $Y" open files not needed for the requested range? I think no. Bisect of master got: 30a53b7 Wed Mar 8 16:56:37 2023 +0100 Allow tailoring of ICU locales with custom rules Doesn't fail at $(git merge-base REL_15_STABLE master). Bisect of v15 got: 811203d Sat Aug 6 11:50:23 2022 -0400 Fix data-corruption hazard in WAL-logged CREATE DATABASE. I suspect those are innocent. They changed the exact WAL content, which I expect somehow caused creation of segment 2. Oddly, I find only one other report of this: https://www.postgresql.org/message-id/CAJ6DU3HiJ5FHbqPua19jAD%3DwLgiXBTjuHfbmv1jCOaNOpB3cCQ%40mail.g... Thanks, nm Attachments: [application/x-perl] 010_zero.pl (545B, ../../[email protected]/2-010_zero.pl) download ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: pg_waldump vs. all-zeros WAL files; server creation of such files 2023-08-13 03:15 pg_waldump vs. all-zeros WAL files; server creation of such files Noah Misch <[email protected]> @ 2023-08-14 01:37 ` Michael Paquier <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Michael Paquier @ 2023-08-14 01:37 UTC (permalink / raw) To: Noah Misch <[email protected]>; +Cc: pgsql-hackers On Sat, Aug 12, 2023 at 08:15:31PM -0700, Noah Misch wrote: > The attached 010_zero.pl, when run as part of the pg_waldump test suite, fails > at today's master (c36b636) and v15 (1bc19df). It passes at v14 (5a32af3). > Command "pg_waldump --start 0/01000000 --end 0/01000100" fails as follows: > > pg_waldump: error: WAL segment size must be a power of two between > 1 MB and 1 GB, but the WAL file "000000010000000000000002" header > specifies 0 bytes So this depends on the ordering of the entries retrieved by readdir() as much as the segments produced by the backend. > Where it fails, the server has created an all-zeros WAL file under that name. > Where it succeeds, that file doesn't exist at all. Two decisions to make: > > - Should a clean server shutdown ever leave an all-zeros WAL file? I think > yes, it's okay to let that happen. It doesn't hurt to leave that around. On the contrary, it makes any follow-up startup cheaper the bigger the segment size. > - Should "pg_waldump --start $X --end $Y" open files not needed for the > requested range? I think no. So this is a case where identify_target_directory() is called with a fname of NULL. Agreed that search_directory could be smarter with the files it should scan, especially if we have start and/or end LSNs at hand to filter out what we'd like to be in the data folder. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-08-14 01:37 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-13 07:58 [PATCH v46 2/7] Add conditional lock feature to dshash Kyotaro Horiguchi <[email protected]> 2023-08-13 03:15 pg_waldump vs. all-zeros WAL files; server creation of such files Noah Misch <[email protected]> 2023-08-14 01:37 ` Re: pg_waldump vs. all-zeros WAL files; server creation of such files Michael Paquier <[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