Re: Use WALReadFromBuffers in more places

Hi Bharath,

I spent some time examining the patch. Here are my observations from the review.

- I believe there’s no need for an extra variable ‘nbytes’ in this
context. We can repurpose the ‘count’ variable for the same function.
If necessary, we might think about renaming ‘count’ to ‘nbytes’.

- The operations performed by logical_read_xlog_page() and
read_local_xlog_page_guts() are identical. It might be beneficial to
create a shared function to minimize code repetition.

Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft

On Mon, May 13, 2024 at 12:17 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Wed, May 8, 2024 at 9:51 AM Jingtang Zhang <mrdrivingduck@gmail.com> wrote:
> >
> > Hi, Bharath. I've been testing this. It's cool. Is there any way we could
> > monitor the hit rate about directly reading from WAL buffers by exporting
> > to some views?
>
> Thanks for looking into this. I used purpose-built patches for
> verifying the WAL buffers hit ratio, please check
> USE-ON-HEAD-Collect-WAL-read-from-file-stats.txt and
> USE-ON-PATCH-Collect-WAL-read-from-buffers-and-file-stats.txt at
> https://www.postgresql.org/message-id/CALj2ACU9cfAcfVsGwUqXMace_7rfSBJ7%2BhXVJfVV1jnspTDGHQ%40mail.gmail.com.
> In the long run, it's better to extend what's proposed in the thread
> https://www.postgresql.org/message-id/CALj2ACU_f5_c8F%2BxyNR4HURjG%3DJziiz07wCpQc%3DAqAJUFh7%2B8w%40mail.gmail.com.
> I haven't had a chance to dive deep into that thread yet, but a quick
> glance over v8 patch tells me that it hasn't yet implemented the idea
> of collecting WAL read stats for both walsenders and the backends
> reading WAL. If that's done, we can extend it for WAL read from WAL
> buffers.
>
> --
> Bharath Rupireddy
> PostgreSQL Contributors Team
> RDS Open Source Databases
> Amazon Web Services: https://aws.amazon.com
>
>