public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v56 2/6] Add conditional lock feature to dshash
4+ messages / 4 participants
[nested] [flat]

* [PATCH v56 2/6] Add conditional lock feature to dshash
@ 2020-03-13 07:58  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 4+ 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 29ad767618..f79b6de245 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_16_10_27_55_2021_500)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v56-0003-Shared-memory-based-stats-collector.patch"



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: documentation structure
@ 2024-07-19 03:06  Tatsuo Ishii <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: Tatsuo Ishii @ 2024-07-19 03:06 UTC (permalink / raw)
  To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; pgsql-hackers

> I'm opposed to having a separate file for every function. I think
> breaking up func.sgml into one piece per sect1 is about right. If that
> proves cumbersome still we can look at breaking it up further, but
> let's start with that.

That will create at least 30 func-xx.sgml files.

t-ishii$ grep '<sect1' func.sgml|wc -l
30

I am afraid that's too many?

Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp






^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: documentation structure
@ 2024-07-19 04:07  David G. Johnston <[email protected]>
  parent: Tatsuo Ishii <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: David G. Johnston @ 2024-07-19 04:07 UTC (permalink / raw)
  To: Tatsuo Ishii <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; pgsql-hackers

On Thu, Jul 18, 2024 at 8:06 PM Tatsuo Ishii <[email protected]> wrote:

> > I'm opposed to having a separate file for every function. I think
> > breaking up func.sgml into one piece per sect1 is about right. If that
> > proves cumbersome still we can look at breaking it up further, but
> > let's start with that.
>
> That will create at least 30 func-xx.sgml files.
>
> t-ishii$ grep '<sect1' func.sgml|wc -l
> 30
>
> I am afraid that's too many?
>
>
The premise and the resultant number of files both seem reasonable to me.
I could get that number down to maybe 20 if pressed but I don't see any
benefit to doing so. I look at a page on the website that needs updating
then go open its source file.  Nice and tidy.

David J.


^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: documentation structure
@ 2024-07-19 06:55  jian he <[email protected]>
  parent: David G. Johnston <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: jian he @ 2024-07-19 06:55 UTC (permalink / raw)
  To: David G. Johnston <[email protected]>; +Cc: Tatsuo Ishii <[email protected]>; [email protected]; [email protected]; [email protected]; pgsql-hackers

On Fri, Jul 19, 2024 at 12:07 PM David G. Johnston
<[email protected]> wrote:
>
> On Thu, Jul 18, 2024 at 8:06 PM Tatsuo Ishii <[email protected]> wrote:
>>
>> > I'm opposed to having a separate file for every function. I think
>> > breaking up func.sgml into one piece per sect1 is about right. If that
>> > proves cumbersome still we can look at breaking it up further, but
>> > let's start with that.
>>
>> That will create at least 30 func-xx.sgml files.
>>
>> t-ishii$ grep '<sect1' func.sgml|wc -l
>> 30
>>
>> I am afraid that's too many?
>>
>
> The premise and the resultant number of files both seem reasonable to me.  I could get that number down to maybe 20 if pressed but I don't see any benefit to doing so. I look at a page on the website that needs updating then go open its source file.  Nice and tidy.
>

hi. my python test.py script in [1] cut func.sgml from 31k lines to
13k lines by putting some contents to these 7 new files.
        new file:   func-admin.sgml
        new file:   func-aggregate.sgml
        new file:   func-datetime.sgml
        new file:   func-info.sgml
        new file:   func-json.sgml
        new file:   func-matching.sgml
        new file:   func-string.sgml

doc/src/sgml/filelist.sgml changes are addressed in the attached patch.
cutting func.sgml into half is ok for me.


in test.py , at line 35, i use
"os.chdir("/home/jian/Desktop/pg_src/src7/postgres/doc/src/sgml")"
you need to change to your corresponding "doc/src/sgml" directory.
overall, you need apply the attached patch, change test.py line 35.



main gotcha is based on pattern mentioned in [2] and
index layout from https://www.postgresql.org/docs/devel/functions.html

[1] https://postgr.es/m/CACJufxH%2BYi521QrncwnW4sFGOhPmJQpsmoJ%2BYnj%2BVpHu5wAahQ%40mail.gmail.com\
[2] http://postgr.es/m/CACJufxEcMjjn-m6fpC2wXHsQbE5nyd%3Dxt6k-jDizBVUKK6O4KQ%40mail.gmail.com


Attachments:

  [text/x-patch] v1-0001-filelist.sgml-add-new-file-entries.patch (1.1K, ../../CACJufxHR1nuLSa_c0=kuk1b0E4Um3-rpyegeWBkG3jn7kKwRCw@mail.gmail.com/2-v1-0001-filelist.sgml-add-new-file-entries.patch)
  download | inline diff:
From cf98a802a7823bf3960b6e08aa1d06f6f944b3fe Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Fri, 19 Jul 2024 14:31:58 +0800
Subject: [PATCH v1 1/1] filelist.sgml add new file entries

---
 doc/src/sgml/filelist.sgml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml
index a7ff5f82..96f81cdc 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -18,6 +18,13 @@
 <!ENTITY ddl        SYSTEM "ddl.sgml">
 <!ENTITY dml        SYSTEM "dml.sgml">
 <!ENTITY func       SYSTEM "func.sgml">
+<!ENTITY func-string       SYSTEM "func-string.sgml">
+<!ENTITY func-matching       SYSTEM "func-matching.sgml">
+<!ENTITY func-datetime       SYSTEM "func-datetime.sgml">
+<!ENTITY func-json       SYSTEM "func-json.sgml">
+<!ENTITY func-aggregate       SYSTEM "func-aggregate.sgml">
+<!ENTITY func-info       SYSTEM "func-info.sgml">
+<!ENTITY func-admin     SYSTEM "func-admin.sgml">
 <!ENTITY indices    SYSTEM "indices.sgml">
 <!ENTITY json       SYSTEM "json.sgml">
 <!ENTITY mvcc       SYSTEM "mvcc.sgml">
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2024-07-19 06:55 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-13 07:58 [PATCH v56 2/6] Add conditional lock feature to dshash Kyotaro Horiguchi <[email protected]>
2024-07-19 03:06 Re: documentation structure Tatsuo Ishii <[email protected]>
2024-07-19 04:07 ` Re: documentation structure David G. Johnston <[email protected]>
2024-07-19 06:55   ` Re: documentation structure jian he <[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