On Wed, May 12, 2021 at 9:08 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, May 12, 2021 at 7:53 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, May 12, 2021 at 4:00 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > >
> > > Ugh, since by commit 592f00f8de we send slot stats every after
> > > spil/stream it’s possible that we report slot stats that have non-zero
> > > counters for spill_bytes/txns and zeroes for total_bytes/txns. It
> > > seems to me it’s legitimate that the slot stats view shows non-zero
> > > values for spill_bytes/txns and zero values for total_bytes/txns
> > > during decoding a large transaction. So I think we can fix the test
> > > script so that it checks only spill_bytes/txns when checking spilled
> > > transactions.
> > >
> >
> > Your analysis and fix look correct to me.
> >
>
> I think the part of the test that tests the stats after resetting it
> might give different results. This can happen because in the previous
> test we spill multiple times (spill_count is 12 in my testing) and it
> is possible that some of the spill stats messages is received by stats
> collector after the reset message. If this theory is correct then it
> better that we remove the test for reset stats and the test after it
> "decode and check stats again.". This is not directly related to your
> patch or buildfarm failure but I guess this can happen and we might
> see such a failure in future.
I agree with your analysis to remove that test. Attached patch has the
changes for the same.
Regards,
Vignesh