| From: | Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> |
|---|---|
| To: | Aditya Gollamudi <adigollamudi(at)gmail(dot)com> |
| Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, michael(at)paquier(dot)xyz, bertranddrouvot(dot)pg(at)gmail(dot)com, andres(at)anarazel(dot)de, shveta(dot)malik(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Report bytes and transactions actually sent downtream |
| Date: | 2026-08-24 09:45:11 |
| Message-ID: | CAE9k0PmtTYcayY5i3oDFazbZy0QqVYYiYqxP9FF-EY8RZcsj_g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Thu, Aug 20, 2026 at 8:32 AM Aditya Gollamudi <adigollamudi(at)gmail(dot)com> wrote:
>
>
> Thanks for the updated patches. I reviewed the latest two as
> part of the July patch review workshop:
>
> First, I wasn't able to cleanly apply the refactoring patch against the
> current tree. It could be an issue on my end, but it may need to be
> checked again.
>
You were right, it needed a rebase. Please find the rebased
refactoring patch (0002*.patch) attached.
> Secondly, this was mentioned earlier in the thread and handled for
> the SQL API path but I believe an excess 25 bytes is still being
> counted by output_bytes in the WALsender path because of an
> unaccounted for header.
>
The patch is intended to track the total number of output bytes
produced by the logical decoding output path. This includes the
payload generated by the output plugin and any message header added by
the output writer. In the walsender path, the output writer always
prepends a WALData message header to each payload generated by the
output plugin. Therefore, the header is included because it forms part
of the complete output message passed to libpq for delivery to the
downstream consumer.
I have updated the documentation to clarify this point. Please let me
know if it remains unclear.
> Finally, I wonder if the test coverage is sufficient. We only
> assert that output_bytes is > 0 and that it is equal to 0 after a reset
> happens. Should we also check that it is equal to the expected
> number of bytes for the output_plugin?
>
Yes, it makes sense to add a test case verifying that the number of
bytes produced by the logical decoding output path matches the
output_bytes value recorded in pg_stat_replication_slots. I have added
one of such test-case in the attached 0001 patch.
Please have a look into the attached patches with above changes and
let me know for any other comments/suggestions.
--
With Regards,
Ashutosh Sharma.
| Attachment | Content-Type | Size |
|---|---|---|
| v20260824-0002-Refactor-replication-slot-decoding-stats-handling.patch | application/octet-stream | 15.2 KB |
| v20260824-0001-Report-output-bytes-in-pg_stat_replication_slots.patch | application/octet-stream | 27.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Tomas Vondra | 2026-08-24 09:38:07 | Re: Changing the state of data checksums in a running cluster |