Out of date comment in predicate.c
Hi hackers,
Commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f got rid of
FirstPredicateLockMgrLock, but it's still referred to in a comment in
predicate.c where the locking protocol is documented. I think it's
probably best to use the name of the macro that's usually used to
access the lock array in the code. Please see attached.
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
fix-comments.patchapplication/octet-stream; name=fix-comments.patchDownload
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index a4cb4d33ad..d48fba40da 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -116,9 +116,9 @@
* than its own active transaction must acquire an exclusive
* lock.
*
- * FirstPredicateLockMgrLock based partition locks
+ * PredicateLockHashPartitionLock(hashcode)
* - The same lock protects a target, all locks on that target, and
- * the linked list of locks on the target..
+ * the linked list of locks on the target.
* - When more than one is needed, acquire in ascending order.
*
* SerializableXactHashLock
On 6/27/17 01:21, Thomas Munro wrote:
Commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f got rid of
FirstPredicateLockMgrLock, but it's still referred to in a comment in
predicate.c where the locking protocol is documented. I think it's
probably best to use the name of the macro that's usually used to
access the lock array in the code. Please see attached.
Does this apply equally to PredicateLockHashPartitionLock() and
PredicateLockHashPartitionLockByIndex()? Should the comment mention or
imply both?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Jul 1, 2017 at 6:38 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 6/27/17 01:21, Thomas Munro wrote:
Commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f got rid of
FirstPredicateLockMgrLock, but it's still referred to in a comment in
predicate.c where the locking protocol is documented. I think it's
probably best to use the name of the macro that's usually used to
access the lock array in the code. Please see attached.Does this apply equally to PredicateLockHashPartitionLock() and
PredicateLockHashPartitionLockByIndex()? Should the comment mention or
imply both?
Yeah, I guess so. How about listing the hashcode variant, as it's the
more commonly used and important for a reader to understand of the
two, but mentioning the ByIndex variant in a bullet point below? Like
this.
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
fix-comments-v2.patchapplication/octet-stream; name=fix-comments-v2.patchDownload
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index a4cb4d33add..009519f56f6 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -116,10 +116,12 @@
* than its own active transaction must acquire an exclusive
* lock.
*
- * FirstPredicateLockMgrLock based partition locks
+ * PredicateLockHashPartitionLock(hashcode)
* - The same lock protects a target, all locks on that target, and
- * the linked list of locks on the target..
- * - When more than one is needed, acquire in ascending order.
+ * the linked list of locks on the target.
+ * - When more than one is needed, acquire in ascending address order.
+ * - When all are needed (rare), acquire in ascending index order with
+ * PredicateLockHashPartitionLockByIndex(index).
*
* SerializableXactHashLock
* - Protects both PredXact and SerializableXidHash.
On 7/6/17 21:06, Thomas Munro wrote:
On Sat, Jul 1, 2017 at 6:38 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 6/27/17 01:21, Thomas Munro wrote:
Commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f got rid of
FirstPredicateLockMgrLock, but it's still referred to in a comment in
predicate.c where the locking protocol is documented. I think it's
probably best to use the name of the macro that's usually used to
access the lock array in the code. Please see attached.Does this apply equally to PredicateLockHashPartitionLock() and
PredicateLockHashPartitionLockByIndex()? Should the comment mention or
imply both?Yeah, I guess so. How about listing the hashcode variant, as it's the
more commonly used and important for a reader to understand of the
two, but mentioning the ByIndex variant in a bullet point below? Like
this.
Committed and backpatched to 9.4.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers