Hi Stas,
> Reading through patch I’ve noticed that you deleted call to > SnapBuildCommitTxn() > in DecodePrepare(). As you correctly spotted upthread there was unnecessary > code that marked transaction as running after decoding of prepare. However > call > marking it as committed before decoding of prepare IMHO is still needed as > SnapBuildCommitTxn does some useful thing like setting base snapshot for > parent > transactions which were skipped because of SnapBuildXactNeedsSkip(). > > E.g. current code will crash in assert for following transaction: > > BEGIN; > SAVEPOINT one; > CREATE TABLE test_prepared_savepoints (a int); > PREPARE TRANSACTION 'x'; > COMMIT PREPARED 'x'; > :get_with2pc_nofilter > :get_with2pc_nofilter <- second call will crash decoder > Thanks for taking a look! The first ":get_with2pc_nofilter" call consumes the data appropriately. The second ":get_with2pc_nofilter" sees that it has to skip and hence enters the ReorderBufferForget() function in the skip code path causing the assert. If we have to skip anyways why do we need to setup SnapBuildCommitTxn() for such a transaction is my query? I don't see the need for doing that for skipped transactions.. Will continue to look at this and will add this scenario to the test cases. Further comments/feedback appreciated. Regards, Nikhils -- Nikhil Sontakke http://www.2ndQuadrant.com/ PostgreSQL/Postgres-XL Development, 24x7 Support, Training & Services
