On Thu, Sep 10, 2020 at 11:53 AM Dilip Kumar <dilipbal...@gmail.com> wrote:
> 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. > I have removed some comments which are not valid after this patch. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
v2-0001-Skip-printing-empty-stream-in-test-decoding.patch
Description: Binary data
