bufmgr: pass through I/O stats context in FlushUnlockedBuffer() - Mailing list pgsql-hackers

From Chao Li
Subject bufmgr: pass through I/O stats context in FlushUnlockedBuffer()
Date
Msg-id BC97546F-5C15-42F2-AD57-CFACDB9657D0@gmail.com
Whole thread Raw
List pgsql-hackers
Hi,

I noticed that FlushUnlockedBuffer() accepts io_object and io_context, but then ignores them and hardcodes
IOOBJECT_RELATIONandIOCONTEXT_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
thecaller. 

[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

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: pg_waldump: support decoding of WAL inside tarfile
Next
From: Amit Kapila
Date:
Subject: Re: Skipping schema changes in publication