Re:Re: BUG #15039: some question about hash index code

thank you for your quick reply.and i have another question, for the following code, whether exist such scene : page_found is false andnewmapbuf is invalid, if so, may be the statement MarkBufferDirty(metabuf); should be placed outside the if statement ?because metap->hashm_spares is changed when page_found is false, buf not mark dirty.

if (page_found)
{
......
}
else
{
/* update the count to indicate new overflow page is added */
metap->hashm_spares[splitnum]++;

if (BufferIsValid(newmapbuf))
{
......

/* add the new bitmap page to the metapage's list of bitmaps */
metap->hashm_mapp[metap->hashm_nmaps] = BufferGetBlockNumber(newmapbuf);
metap->hashm_nmaps++;
metap->hashm_spares[splitnum]++;
MarkBufferDirty(metabuf);
}
}

At 2018-01-31 20:06:22, "Amit Kapila" <amit(dot)kapila16(at)gmail(dot)com> wrote:
>On Wed, Jan 31, 2018 at 5:04 PM, PG Bug reporting form
><noreply(at)postgresql(dot)org> wrote:
>> The following bug has been logged on the website:
>>
>> Bug reference: 15039
>> Logged by: lixian zou
>> Email address: zoulx1982(at)163(dot)com
>> PostgreSQL version: 10.0
>> Operating system: source code
>> Description:
>>
>> hi,
>> i read hash index code , and found in _hash_addovflpage function, there is
>> such code :
>> if (metap->hashm_firstfree == orig_firstfree)
>> {
>> metap->hashm_firstfree = bit + 1;
>> MarkBufferDirty(metabuf);
>> }
>>
>> i found no any chang for metap,metap->hashm_firstfree,and initial the two
>> variable is equal, so maybe the if statement is redundant?
>>
>
>The hashm_firstfree can be changed by a concurrent session as we
>release and reacquire the lock on a metapage while trying to get the
>bitmap page. See, below code:
>
>_hash_addovflpage
>{
>..
>..
>
>for (;;)
>{
>..
>/* Release exclusive lock on metapage while reading bitmap page */
>LockBuffer(metabuf, BUFFER_LOCK_UNLOCK);
>..
>
>/* Reacquire exclusive lock on the meta page */
>LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE);
>}
>..
>
>if (metap->hashm_firstfree == orig_firstfree)
>{
>metap->hashm_firstfree = bit + 1;
>MarkBufferDirty(metabuf);
>}
>..
>}
>
>
>I think pgsql-hackers is the better place to get clarifications related to code.
>
>--
>With Regards,
>Amit Kapila.
>EnterpriseDB: http://www.enterprisedb.com