On Wed, Feb 8, 2023 at 10:33 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Wed, Feb 8, 2023 at 9:57 AM Bharath Rupireddy > <bharath.rupireddyforpostg...@gmail.com> wrote: > > > > I can also do a few other things, but before working on them, I'd like > > to hear from others: > > 1. A separate wait event (WAIT_EVENT_WAL_READ_FROM_BUFFERS) for > > reading from WAL buffers - right now, WAIT_EVENT_WAL_READ is being > > used both for reading from WAL buffers and WAL files. Given the fact > > that we won't wait for a lock or do a time-taking task while reading > > from buffers, it seems unnecessary. > > Yes, we do not need this separate wait event and we also don't need > WAIT_EVENT_WAL_READ wait event while reading from the buffer. Because > we are not performing any IO so no specific wait event is needed and > for reading from the WAL buffer we are acquiring WALBufMappingLock so > that lwlock event will be tracked under that lock.
Nope, LWLockConditionalAcquire doesn't wait, so no lock wait event (no LWLockReportWaitStart) there. I agree to not have any wait event for reading from WAL buffers as no IO is involved there. I removed it in the attached v4 patch. > > 2. A separate TAP test for verifying that the WAL is actually read > > from WAL buffers - right now, existing tests for recovery, > > subscription, pg_walinspect already cover the code, see [1]. However, > > if needed, I can add a separate TAP test. > > Can we write a test that can actually validate that we have read from > a WAL Buffer? If so then it would be good to have such a test to avoid > any future breakage in that logic. But if it is just for hitting the > code but no guarantee that whether we can validate as part of the test > whether it has hit the WAL buffer or not then I think the existing > cases are sufficient. We could set up a standby or a logical replication subscriber or pg_walinspect extension and verify if the code got hit with the help of the server log (DEBUG1) message added by the patch. However, this can make the test volatile. Therefore, I came up with a simple and small test module/extension named test_wal_read_from_buffers under src/test/module. It basically exposes a SQL-function given an LSN, it calls XLogReadFromBuffers() and returns true if it hits WAL buffers, otherwise false. And the simple TAP test of this module verifies if the function returns true. I attached the test module as v4-0002 here. The test module looks specific and also helps as demonstration of how one can possibly use the new XLogReadFromBuffers(). Thoughts? -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
v4-0001-Improve-WALRead-to-suck-data-directly-from-WAL-bu.patch
Description: Binary data
v4-0002-Add-test-module-for-verifying-WAL-read-from-WAL-b.patch
Description: Binary data