On Wed, May 12, 2021 at 1:19 PM vignesh C <vignes...@gmail.com> wrote: > > On Wed, May 12, 2021 at 9:08 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Wed, May 12, 2021 at 7:53 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > On Wed, May 12, 2021 at 4:00 AM Masahiko Sawada <sawada.m...@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.
Good point. I agree to remove this test. > > I agree with your analysis to remove that test. Attached patch has the > changes for the same. Thank you for the patch. The patch looks good to me. I also agree that removing the test doesn't reduce the test coverage. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
