From: | "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com> |
---|---|
To: | "k(dot)jamison(at)fujitsu(dot)com" <k(dot)jamison(at)fujitsu(dot)com> |
Cc: | 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | RE: [Patch] Optimize dropping of relation buffers using dlist |
Date: | 2020-09-08 05:49:21 |
Message-ID: | TYAPR01MB2990472119300345A2B63235FE290@TYAPR01MB2990.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> if (RelFileNodeEquals(bufHdr->tag.rnode, rnode.node) &&
> + bufHdr->tag.forkNum == forkNum[j] &&
> + bufHdr->tag.blockNum >= firstDelBlock[j])
>
> Here, I think you need to use 'i' not 'j' for forkNum and
> firstDelBlock as those are arrays w.r.t forks. That might fix the
> problem but I am not sure as I haven't tried to reproduce it.
(1)
+ INIT_BUFFERTAG(newTag, rnode.node, forkNum[j], firstDelBlock[j]);
And you need to use i here, too.
I advise you to suspect any character, any word, and any sentence. I've found many bugs for others so far. I'm afraid you're just seeing the code flow.
(2)
+ LWLockAcquire(newPartitionLock, LW_SHARED);
+ buf_id = BufTableLookup(&newTag, newHash);
+ LWLockRelease(newPartitionLock);
+
+ bufHdr = GetBufferDescriptor(buf_id);
Check the result of BufTableLookup() and do nothing if the block is not in the shared buffers.
(3)
+ else
+ {
+ for (j = BUF_DROP_FULLSCAN_THRESHOLD; j < NBuffers; j++)
+ {
What's the meaning of this loop? I don't understand the start condition. Should j be initialized to 0?
(4)
+#define BUF_DROP_FULLSCAN_THRESHOLD (NBuffers / 2)
Wasn't it 500 instead of 2? Anyway, I think we need to discuss this threshold later.
(5)
+ if (((int)nblocks) < BUF_DROP_FULLSCAN_THRESHOLD)
It's better to define BUF_DROP_FULLSCAN_THRESHOLD as an uint32 value instead of casting the type here, as these values are blocks.
Regards
Takayuki Tsunakawa
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Khandekar | 2020-09-08 05:50:06 | Re: Auto-vectorization speeds up multiplication of large-precision numerics |
Previous Message | Pavel Stehule | 2020-09-08 05:45:43 | Re: Reduce the time required for a database recovery from archive. |