Hi! On Wed, Jun 21, 2017 at 10:52 AM, Shubham Barai <shubhambara...@gmail.com> wrote:
> Hi, > > On 21 June 2017 at 13:11, Heikki Linnakangas <hlinn...@iki.fi> wrote: > >> On 06/16/2017 01:24 PM, Shubham Barai wrote: >> >>> @@ -497,6 +499,13 @@ gistplacetopage(Relation rel, Size freespace, >>> GISTSTATE *giststate, >>> for (ptr = dist->next; ptr; ptr = ptr->next) >>> UnlockReleaseBuffer(ptr->buffer); >>> } >>> + >>> + for (ptr = dist; ptr; ptr = ptr->next) >>> + PredicateLockPageSplit(rel, >>> + >>> BufferGetBlockNumber(buffer), >>> + >>> BufferGetBlockNumber(ptr->buffer)); >>> + >>> + >>> >> >> I think this new code needs to go before the UnlockReleaseBuffer() calls >> above. Calling BufferGetBlockNumber() on an already-released buffer is not >> cool. >> >> - Heikki >> >> I know that. This is the old version of the patch. I had sent updated > patch later. Please have a look at updated patch. > I took a look on this patch. In gistdoinsert() you do CheckForSerializableConflictIn() only if page wasn't exclusively locked before (xlocked is false). if (!xlocked) > { > LockBuffer(stack->buffer, GIST_UNLOCK); > LockBuffer(stack->buffer, GIST_EXCLUSIVE); > CheckForSerializableConflictIn(r, NULL, stack->buffer); > xlocked = true; However, page might be exclusively locked before. And in this case CheckForSerializableConflictIn() would be skipped. That happens very rarely (someone fixes incomplete split before we did), but nevertheless. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company