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.kapila16@gmail.com> wrote:
>On Wed, Jan 31, 2018 at 5:04 PM, PG Bug reporting form
><noreply@postgresql.org> wrote:
>> The following bug has been logged on the website:
>>
>> Bug reference:      15039
>> Logged by:          lixian zou
>> Email address:      zoulx1982@163.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