On Thu, Sep 24, 2020 at 5:44 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sat, Sep 19, 2020 at 1:48 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Tue, Sep 8, 2020 at 7:02 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Tue, Sep 8, 2020 at 7:53 AM Masahiko Sawada
> > > <masahiko.sawada@2ndquadrant.com> wrote:
> >
> > I have fixed these review comments in the attached patch.
> >
> >
> > Apart from the above,
> > (a) fixed one bug in ReorderBufferSerializeTXN() where we were
> > updating the stats even when we have not spilled anything.
> > (b) made changes in pgstat_read_db_statsfile_timestamp to return false
> > when the replication slot entry is corrupt.
> > (c) move the declaration and definitions in pgstat.c to make them
> > consistent with existing code
> > (d) made another couple of cosmetic fixes and changed a few comments
> > (e) Tested the patch by using a guc which allows spilling all the
> > changes. See v4-0001-guc-always-spill
> >
>
> I have found a way to write the test case for this patch. This is
> based on the idea we used in stats.sql. As of now, I have kept the
> test as a separate patch. We can decide to commit the test part
> separately as it is slightly timing dependent but OTOH as it is based
> on existing logic in stats.sql so there shouldn't be much problem. I
> have not changed anything apart from the test patch in this version.
> Note that the first patch is just a debugging kind of tool to test the
> patch.
>
I have done some more testing of this patch especially for the case
where we spill before streaming the transaction and found everything
is working as expected. Additionally, I have changed a few more
comments and ran pgindent. I am still not very sure whether we want to
display physical slots in this view as all the stats are for logical
slots but anyway we can add stats w.r.t physical slots in the future.
I am fine either way (don't show physical slots in this view or show
them but keep stats as 0). Let me know if you have any thoughts on
these points, other than that I am happy with the current state of the
patch.
--
With Regards,
Amit Kapila.
