| Lists: | pgsql-hackers |
|---|
| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Cc: | Andres Freund <andres(at)anarazel(dot)de>, Melanie Plageman <melanieplageman(at)gmail(dot)com> |
| Subject: | bufmgr: pass through I/O stats context in FlushUnlockedBuffer() |
| Date: | 2026-04-01 02:15:09 |
| Message-ID: | BC97546F-5C15-42F2-AD57-CFACDB9657D0@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hi,
I noticed that FlushUnlockedBuffer() accepts io_object and io_context, but then ignores them and hardcodes IOOBJECT_RELATIONand IOCONTEXT_NORMAL instead:
```
static void
FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
IOObject io_object, IOContext io_context)
{
Buffer buffer = BufferDescriptorGetBuffer(buf);
BufferLockAcquire(buffer, buf, BUFFER_LOCK_SHARE_EXCLUSIVE);
FlushBuffer(buf, reln, IOOBJECT_RELATION, IOCONTEXT_NORMAL); // <== HERE
BufferLockUnlock(buffer, buf);
}
```
Unless I am missing something, if a function accepts these parameters, they should generally be used.
FlushBuffer() seems to have the same issue. It takes both io_object and io_context:
```
static void
FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object,
IOContext io_context)
```
but while io_context is used, io_object is ignored:
```
pgstat_count_io_op_time(IOOBJECT_RELATION, io_context,
IOOP_WRITE, io_start, 1, BLCKSZ);
```
For comparison, in AsyncReadBuffers(), where io_object is also available locally, it is passed through and used directly:
```
pgstat_count_io_op_time(io_object, io_context, IOOP_READ,
io_start, 1, io_buffers_len * BLCKSZ);
```
I raised the same point while reviewing patch [1], but that patch has not been updated yet.
This tiny patch just makes FlushUnlockedBuffer() and FlushBuffer() use the io_object and io_context values passed in by the caller.
[1] Discussion: https://postgr.es/m/377BC880-1616-4DEF-B9EF-5E297C358F7D@gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-bufmgr-pass-through-I-O-stats-context-in-FlushUnl.patch | application/octet-stream | 1.8 KB |
| From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de> |
| Subject: | Re: bufmgr: pass through I/O stats context in FlushUnlockedBuffer() |
| Date: | 2026-04-21 21:52:35 |
| Message-ID: | CAAKRu_aUfeVKFW34ZvuhY9tBXUKYfn8rs-8cf-aE4YSUFj-xEQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On Tue, Mar 31, 2026 at 10:15 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
> I noticed that FlushUnlockedBuffer() accepts io_object and io_context, but then ignores them and hardcodes IOOBJECT_RELATIONand IOCONTEXT_NORMAL instead:
> ```
> static void
> FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
> IOObject io_object, IOContext io_context)
> {
> Buffer buffer = BufferDescriptorGetBuffer(buf);
>
> BufferLockAcquire(buffer, buf, BUFFER_LOCK_SHARE_EXCLUSIVE);
> FlushBuffer(buf, reln, IOOBJECT_RELATION, IOCONTEXT_NORMAL); // <== HERE
> BufferLockUnlock(buffer, buf);
> }
> ```
>
> Unless I am missing something, if a function accepts these parameters, they should generally be used.
Thanks for the patch. Committed in 31b0544b32b
- Melanie
| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de> |
| Subject: | Re: bufmgr: pass through I/O stats context in FlushUnlockedBuffer() |
| Date: | 2026-04-21 22:24:04 |
| Message-ID: | 275CB553-A9F6-4C4D-92FD-78DCC139205F@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Lists: | pgsql-hackers |
> On Apr 22, 2026, at 05:52, Melanie Plageman <melanieplageman(at)gmail(dot)com> wrote:
>
> On Tue, Mar 31, 2026 at 10:15 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>> I noticed that FlushUnlockedBuffer() accepts io_object and io_context, but then ignores them and hardcodes IOOBJECT_RELATIONand IOCONTEXT_NORMAL instead:
>> ```
>> static void
>> FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
>> IOObject io_object, IOContext io_context)
>> {
>> Buffer buffer = BufferDescriptorGetBuffer(buf);
>>
>> BufferLockAcquire(buffer, buf, BUFFER_LOCK_SHARE_EXCLUSIVE);
>> FlushBuffer(buf, reln, IOOBJECT_RELATION, IOCONTEXT_NORMAL); // <== HERE
>> BufferLockUnlock(buffer, buf);
>> }
>> ```
>>
>> Unless I am missing something, if a function accepts these parameters, they should generally be used.
>
> Thanks for the patch. Committed in 31b0544b32b
>
> - Melanie
Hi Melanie, thank you very much for pushing.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/