On Thu, Sep 11, 2025 at 2:29 PM Zhijie Hou (Fujitsu) <[email protected]> wrote: > > On Monday, September 8, 2025 7:21 PM Zhijie Hou (Fujitsu) > <[email protected]> wrote: > > > > On Monday, September 8, 2025 3:13 PM Amit Kapila > > <[email protected]> wrote: > > > > > > On Fri, Sep 5, 2025 at 5:03 PM Zhijie Hou (Fujitsu) > > > <[email protected]> > > > wrote: > > > > > > > > Here are v2 patches which addressed above comments. > > > > > > > > > > I have pushed the first patch. I find that the test can't reliably fail > > > without a fix. > > > Can you please investigate it? > > > > Thank you for catching this issue. I confirmed that the test may have tested > > VACCUM before slot.xmin was advanced. Therefore, to improve the test, I > > modified test to wait for the publisher's request message appearing twice, > > as > > after the fix, the apply worker should keep waiting for publisher status > > until the > > prepared txn is committed. > > > > Also, to reduce test time, I moved the test into the existing 035 test. > > > > Here is the updated test. > > I noticed a BF failure[1] on this test. The log shows that the apply worker > advances the non-removable xid to the latest state before waiting for the > prepared transaction to commit. Upon reviewing the log, I didn't find any > clues > of a bug in the code. One potential explanation is that the prepared > transaction > hasn't reached the injection point before the apply worker requests the > publisher status. > > The log lacks the timing for when the injection point is triggered and only > includes: > > pub: 2025-09-11 03:40:05.667 CEST [396867][client backend][8/3:0] LOG: > statement: COMMIT PREPARED 'txn_with_later_commit_ts'; > .. > sub: 2025-09-11 03:40:05.684 CEST [396798][logical replication apply > worker][16/0:0] DEBUG: sending publisher status request message > > Although the statement on the publisher appears before the publisher request, > the statement log is generated prior to command execution. Thus, it's possible > the injection point is triggered after responding to the publisher status. > > After checking some other tap tests using injection points, most of them > ensure > the injection is triggered before proceeding with the test (by waiting for the > wait event of injection point). We could also add this in the test: > > $node_B->wait_for_event('client backend', 'commit-after-delay-checkpoint'); > > Here is a small patch. >
Agree with the analysis. The patch looks good. thanks Shveta
