public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v45 2/7] Add conditional lock feature to dshash
3+ messages / 3 participants
[nested] [flat]
* [PATCH v45 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(Fri_Jan__8_10_24_34_2021_185)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v45-0003-Make-archiver-process-an-auxiliary-process.patch"
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: doc issues in event-trigger-matrix.html
@ 2024-03-19 01:14 Michael Paquier <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Michael Paquier @ 2024-03-19 01:14 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Tue, Mar 19, 2024 at 08:00:00AM +0800, jian he wrote:
> I think the "X" and "-" mean in this matrix [0] is not very intuitive.
> mainly because "X" tends to mean negative things in most cases.
> we can write a sentence saying "X" means this, "-" means that.
>
> or maybe Check mark [1] and Cross mark [2] are more universal.
> and we can use these marks.
>
> "Only for local objects"
> is there any reference explaining "local objects"?
> I think local object means objects that only affect one single database?
It is true that in Japan the cross mark refers to a negation, and
that's the opposite in France: I would put a cross on a table in the
case where something is supported. I've never seen anybody complain
about the format of these tables, FWIW, but if these were to be
changed, the update should happen across the board for all the tables
and not only one.
Using ASCII characters also has the advantage to make the maintenance
clightly easier, in my opinion, because there is no translation effort
between these special characters and their XML equivalent, like "<"
<-> "<".
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: doc issues in event-trigger-matrix.html
@ 2024-03-19 09:34 Daniel Gustafsson <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Gustafsson @ 2024-03-19 09:34 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: jian he <[email protected]>; PostgreSQL Hackers <[email protected]>
> On 19 Mar 2024, at 02:14, Michael Paquier <[email protected]> wrote:
>
> On Tue, Mar 19, 2024 at 08:00:00AM +0800, jian he wrote:
>> I think the "X" and "-" mean in this matrix [0] is not very intuitive.
>> mainly because "X" tends to mean negative things in most cases.
>> we can write a sentence saying "X" means this, "-" means that.
>>
>> or maybe Check mark [1] and Cross mark [2] are more universal.
>> and we can use these marks.
>>
>> "Only for local objects"
>> is there any reference explaining "local objects"?
>> I think local object means objects that only affect one single database?
That's a bigger problem than the table representation, we never define what
"local object" mean anywhere in the EVT docs. EV's are global for a database,
but not a cluster, so I assume what this means is that EVs for non-DDL commands
like COMMENT can only fire for a specific relation they are attached to and not
database wide?
> It is true that in Japan the cross mark refers to a negation, and
> that's the opposite in France: I would put a cross on a table in the
> case where something is supported. I've never seen anybody complain
> about the format of these tables, FWIW, but if these were to be
> changed, the update should happen across the board for all the tables
> and not only one.
AFAICT we only have one other table with "X" denoting support, the "Conflicting
Lock Modes" table under Concurrency Control chapter, and there we simply leave
the "not supported" column empty instead of using a dash. Maybe the simple fix
here is to make these tables consistent by removing the dash from the event
trigger firing matrix?
As a sidenote, the table should gain a sentence explaining why the login column
is missing to avoid confusion.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2024-03-19 09:34 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 v45 2/7] Add conditional lock feature to dshash Kyotaro Horiguchi <[email protected]>
2024-03-19 01:14 Re: doc issues in event-trigger-matrix.html Michael Paquier <[email protected]>
2024-03-19 09:34 ` Re: doc issues in event-trigger-matrix.html Daniel Gustafsson <[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