public inbox for [email protected]  

help / color / mirror / Atom feed

From: Nitin Jadhav <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: Jingtang Zhang <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Use WALReadFromBuffers in more places
Date: Sat, 8 Jun 2024 17:24:17 +0530
Message-ID: <CAMm1aWYa1fGKcuG69xGJPNXLQ_9zHrPqhr-ZGdj4so6Exq66MQ@mail.gmail.com> (raw)
In-Reply-To: <CALj2ACVzL4uU=hxFpSfkqP4HeFCPucbBTEg6HNf_MPTYm52pHg@mail.gmail.com>
References: <CALj2ACVfF2Uj9NoFy-5m98HNtjHpuD17EDE9twVeJng-jTAe7A@mail.gmail.com>
	<CAPsk3_CzLbMe-D07H5Vo6yWFvyXHh5is7AoPUCFcztrUmf1haw@mail.gmail.com>
	<CALj2ACVzL4uU=hxFpSfkqP4HeFCPucbBTEg6HNf_MPTYm52pHg@mail.gmail.com>

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
<[email protected]> wrote:
>
> On Wed, May 8, 2024 at 9:51 AM Jingtang Zhang <[email protected]> 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.g....
> 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%40....
> 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
>
>

view thread (11+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Use WALReadFromBuffers in more places
  In-Reply-To: <CAMm1aWYa1fGKcuG69xGJPNXLQ_9zHrPqhr-ZGdj4so6Exq66MQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox