Thread: BUFFER_LOCK_EXCLUSIVE is used in ginbuildempty().
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 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();
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
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
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
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