BUFFER_LOCK_EXCLUSIVE is used in ginbuildempty().

Started by Kyotaro HORIGUCHIover 11 years ago5 messages
#1Kyotaro HORIGUCHI
horiguchi.kyotaro@lab.ntt.co.jp
1 attachment(s)

Hello,

As far as I see gin seems using GIN_EXCLUSIVE instead of
BUFFER_LOCK_EXCLUSIVE for LockBuffer, but the raw
BUFFER_LOCK_EXCLUSIVE appears in ginbuildempty().

Does it has a meaning to fix them to GIN_EXCLUSIVE?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

gin_buffer_lock_exclusive.patchtext/x-patch; charset=us-asciiDownload
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index b27cae3..004b3a9 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -442,10 +442,10 @@ ginbuildempty(PG_FUNCTION_ARGS)
 	/* An empty GIN index has two pages. */
 	MetaBuffer =
 		ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
-	LockBuffer(MetaBuffer, BUFFER_LOCK_EXCLUSIVE);
+	LockBuffer(MetaBuffer, GIN_EXCLUSIVE);
 	RootBuffer =
 		ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
-	LockBuffer(RootBuffer, BUFFER_LOCK_EXCLUSIVE);
+	LockBuffer(RootBuffer, GIN_EXCLUSIVE);
 
 	/* Initialize and xlog metabuffer and root buffer. */
 	START_CRIT_SECTION();
#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Kyotaro HORIGUCHI (#1)
Re: BUFFER_LOCK_EXCLUSIVE is used in ginbuildempty().

Kyotaro HORIGUCHI wrote:

Hello,

As far as I see gin seems using GIN_EXCLUSIVE instead of
BUFFER_LOCK_EXCLUSIVE for LockBuffer, but the raw
BUFFER_LOCK_EXCLUSIVE appears in ginbuildempty().

Does it has a meaning to fix them to GIN_EXCLUSIVE?

I don't understand the point of having these GIN_EXCLUSIVE / GIN_SHARED
symbols. It's not like we could do anything different than
BUFFER_LOCK_EXCLUSIVE etc instead. It there was a GinLockBuffer() it
might make more sense to have specialized symbols, but as it is it seems
pointless.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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

#3Peter Geoghegan
pg@heroku.com
In reply to: Alvaro Herrera (#2)
Re: BUFFER_LOCK_EXCLUSIVE is used in ginbuildempty().

On Thu, Jul 17, 2014 at 7:47 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

I don't understand the point of having these GIN_EXCLUSIVE / GIN_SHARED
symbols. It's not like we could do anything different than
BUFFER_LOCK_EXCLUSIVE etc instead. It there was a GinLockBuffer() it
might make more sense to have specialized symbols, but as it is it seems
pointless.

It's a pattern common to the index AMs. I think it's kind of pointless
myself, but as long as we're doing it we might as well be consistent.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Geoghegan (#3)
Re: BUFFER_LOCK_EXCLUSIVE is used in ginbuildempty().

Peter Geoghegan <pg@heroku.com> writes:

On Thu, Jul 17, 2014 at 7:47 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

I don't understand the point of having these GIN_EXCLUSIVE / GIN_SHARED
symbols. It's not like we could do anything different than
BUFFER_LOCK_EXCLUSIVE etc instead. It there was a GinLockBuffer() it
might make more sense to have specialized symbols, but as it is it seems
pointless.

It's a pattern common to the index AMs. I think it's kind of pointless
myself, but as long as we're doing it we might as well be consistent.

I think that to the extent that these symbols are used in APIs above the
direct buffer-access layer, they are useful --- for example using
BT_READ/BT_WRITE in _bt_search calls seems like a useful increment of
readability. GIN seems to have less of that than some of the other AMs,
but I do see GIN_SHARE being used that way in some calls.

BTW, there's one direct usage of BUFFER_LOCK_EXCLUSIVE in the GIST code
as well, which should probably be replaced with GIST_EXCLUSIVE if we're
trying to be consistent.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Kyotaro HORIGUCHI
horiguchi.kyotaro@lab.ntt.co.jp
In reply to: Tom Lane (#4)
Re: BUFFER_LOCK_EXCLUSIVE is used in ginbuildempty().

Hello,

At Thu, 17 Jul 2014 15:54:31 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <10710.1405626871@sss.pgh.pa.us>

Peter Geoghegan <pg@heroku.com> writes:

On Thu, Jul 17, 2014 at 7:47 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

I don't understand the point of having these GIN_EXCLUSIVE / GIN_SHARED
symbols. It's not like we could do anything different than
BUFFER_LOCK_EXCLUSIVE etc instead. It there was a GinLockBuffer() it
might make more sense to have specialized symbols, but as it is it seems
pointless.

I agree with you. From the eyes not specialized for each AM, of
me, the translation-only symbols didn't make me so happy.

It's a pattern common to the index AMs. I think it's kind of pointless
myself, but as long as we're doing it we might as well be consistent.

I think that to the extent that these symbols are used in APIs above the
direct buffer-access layer, they are useful --- for example using
BT_READ/BT_WRITE in _bt_search calls seems like a useful increment of
readability. GIN seems to have less of that than some of the other AMs,
but I do see GIN_SHARE being used that way in some calls.

BTW, there's one direct usage of BUFFER_LOCK_EXCLUSIVE in the GIST code
as well, which should probably be replaced with GIST_EXCLUSIVE if we're
trying to be consistent.

Though I brought up this topic, this kind of consistency seems
not needed so much. If so, it seems better to be left as it is.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers