On Thu, Sep 10, 2020 at 11:47 AM Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Thu, Sep 10, 2020 at 11:42 AM Dilip Kumar <dilipbal...@gmail.com> > wrote: > > > > On Thu, Sep 10, 2020 at 11:29 AM Amit Kapila <amit.kapil...@gmail.com> > wrote: > >> > >> Hi, > >> > >> There is a recent build farm failure [1] in one of the test_decoding > >> tests as pointed by Tom Lane [2]. The failure report is shown below: > >> > >> @@ -71,6 +71,8 @@ > >> data > >> ------------------------------------------ > >> opening a streamed block for transaction > >> + closing a streamed block for transaction > >> + opening a streamed block for transaction > >> streaming change for transaction > >> streaming change for transaction > >> streaming change for transaction > >> @@ -83,7 +85,7 @@ > >> streaming change for transaction > >> closing a streamed block for transaction > >> committing streamed transaction > >> -(13 rows) > >> +(15 rows) > >> > >> Here, the symptoms are quite similar to what we have fixed in commit > >> 82a0ba7707 which is that an extra empty transaction is being decoded > >> in the test. It can happen even if have instructed the test to 'skip > >> empty xacts' for streaming transactions because the test_decoding > >> plugin APIs (related to streaming changes for in-progress xacts) makes > >> no effort to skip such empty xacts. It was kept intentionally like > >> that under the assumption that we would never try to stream empty > >> xacts but on closer inspection of the code, it seems to me that > >> assumption was not correct. Basically, we can pick to stream a > >> transaction that has change messages for > >> REORDER_BUFFER_CHANGE_INTERNAL_SNAPSHOT and we don't send such > >> messages to downstream rather they are just to update the internal > >> state. So, in this particular failure, it is possible that autovacuum > >> transaction has got such a change message added by one of the other > >> committed xact and on trying to stream it we get such additional > >> messages. The fix is to skip empty xacts when indicated by the user in > >> streaming APIs of test_decoding. > >> > >> Thoughts? > > > > > > Yeah, that's an issue. > > > >> > >> > >> [1] - > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2020-09-09+03%3A42%3A19 > >> [2] - > https://www.postgresql.org/message-id/118303.1599691636%40sss.pgh.pa.us > >> > > > > I have written a test case to reproduce the same. I have also prepared > a patch to skip the empty transaction. And after that, the issue has been > fixed. But the extra side effect will be that it would skip any empty > stream even if the transaction is not empty. As such I don't see any > problem with that but this is not what the user has asked for. > > > > Isn't that true for non-streaming xacts as well? Basically > skip-empty-xacts option indicates that if there is no change for > 'tuple' or 'message', we skip it. > Yeah, that's right. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
