Re: pgstattuple: Use streaming read API in pgstatindex functions
| От | wenhui qiu |
|---|---|
| Тема | Re: pgstattuple: Use streaming read API in pgstatindex functions |
| Дата | |
| Msg-id | CAGjGUAJ59WCdr5ED=5fV2bpD-PGQGJJhSjckVxw=FFdDpnFySg@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: pgstattuple: Use streaming read API in pgstatindex functions (Xuneng Zhou <xunengzhou@gmail.com>) |
| Список | pgsql-hackers |
HI Xuneng Zhou
> - /* Unlock and release buffer */
> - LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
> - ReleaseBuffer(buffer);
> + UnlockReleaseBuffer(buffer);
> }
> - LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
> - ReleaseBuffer(buffer);
> + UnlockReleaseBuffer(buffer);
> }
The previous suggestion to keep it was based on the fact that the original code already had a similar comment.
In fact, the code itself is quite easy to understand. My earlier email was simply following the style of the existing code when making the suggestion.
If anyone thinks the comment is unnecessary, it can certainly be removed.
In fact, the code itself is quite easy to understand. My earlier email was simply following the style of the existing code when making the suggestion.
If anyone thinks the comment is unnecessary, it can certainly be removed.
On Thu, Oct 16, 2025 at 5:39 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
Hi Kato-san,
Thanks for looking into this.
On Thu, Oct 16, 2025 at 4:21 PM Shinya Kato <shinya11.kato@gmail.com> wrote:
>
> Hi,
>
> On Wed, Oct 15, 2025 at 10:25 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
>>
>> Hi,
>>
>> Here’s the updated summary report(cold cache, fragmented index), now including results for the streaming I/O + io_uring configuration.
>>
>
> Thank you for the additional tests. I can see the image on Gmail, but I cannot on pgsql-hackers archive [0], so it might be a good idea to attach it and not to paste it on the body.
Please see the attachment.
>
>
> I saw the patch and have a few minor comments.
>
> + p.current_blocknum = 1;
>
> To improve readability, how about using the following, which is consistent with nbtree.c [1]?
> p.current_blocknum = BTREE_METAPAGE + 1;
>
> Similarly, for hash index:
> p.current_blocknum = HASH_METAPAGE + 1;
This is more readable. I made the replacements.
>
> + /* Unlock and release buffer */
> UnlockReleaseBuffer(buf);
>
> I think this comment is redundant since the function name UnlockReleaseBuffer is self-explanatory. I suggest omitting it from pgstathashindex and removing the existing one from pgstatindex_impl.
UnlockReleaseBuffer seems clearer and simpler than the original
> LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
> ReleaseBuffer(buffer);
So the comment might be less meaningful for UnlockReleaseBuffer. I
removed it as you suggested.
Best,
Xuneng
В списке pgsql-hackers по дате отправления: