Re: Replication slot stats misgivings

On Tue, Apr 13, 2021 at 1:37 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Mon, Apr 12, 2021 at 7:03 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> >
> > The following test for the latest v8 patch seems to show different.
> > total_bytes is 1808 whereas spill_bytes is 13200000. Am I missing
> > something?
> >
> > postgres(1:85969)=# select pg_create_logical_replication_slot('s',
> > 'test_decoding');
> >  pg_create_logical_replication_slot
> > ------------------------------------
> >  (s,0/1884468)
> > (1 row)
> >
> > postgres(1:85969)=# create table a (i int);
> > CREATE TABLE
> > postgres(1:85969)=# insert into a select generate_series(1, 100000);
> > INSERT 0 100000
> > postgres(1:85969)=# set logical_decoding_work_mem to 64;
> > SET
> > postgres(1:85969)=# select * from pg_stat_replication_slots ;
> >  slot_name | total_txns | total_bytes | spill_txns | spill_count |
> > spill_bytes | stream_txns | stream_count | stream_bytes | stats_reset
> > -----------+------------+-------------+------------+-------------+-------------+-------------+--------------+--------------+-------------
> >  s         |          0 |           0 |          0 |           0 |
> >       0 |           0 |            0 |            0 |
> > (1 row)
> >
> > postgres(1:85969)=# select count(*) from
> > pg_logical_slot_peek_changes('s', NULL, NULL);
> >  count
> > --------
> >  100004
> > (1 row)
> >
> > postgres(1:85969)=# select * from pg_stat_replication_slots ;
> >  slot_name | total_txns | total_bytes | spill_txns | spill_count |
> > spill_bytes | stream_txns | stream_count | stream_bytes | stats_reset
> > -----------+------------+-------------+------------+-------------+-------------+-------------+--------------+--------------+-------------
> >  s         |          2 |        1808 |          1 |         202 |
> > 13200000 |           0 |            0 |            0 |
> > (1 row)
> >
>
> Thanks for identifying this issue, while spilling the transactions
> reorder buffer changes gets released, we will not be able to get the
> total size for spilled transactions from reorderbuffer size. I have
> fixed it by including spilledbytes to totalbytes in case of spilled
> transactions. Attached patch has the fix for this.
> Thoughts?
>

I am not sure if that is the best way to fix it because sometimes we
clear the serialized flag in which case it might not give the correct
answer. Another way to fix it could be that before we try to restore a
new set of changes, we update totalBytes counter. See, the attached
patch atop your v6-0002-* patch.

-- 
With Regards,
Amit Kapila.