Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16952 )
Change subject: KUDU-2612: background task to commit transaction ...................................................................... Patch Set 15: Code-Review+2 (5 comments) Looks good to me! Maybe, Hao has more feedback? http://gerrit.cloudera.org:8080/#/c/16952/7/src/kudu/integration-tests/txn_commit-itest.cc File src/kudu/integration-tests/txn_commit-itest.cc: http://gerrit.cloudera.org:8080/#/c/16952/7/src/kudu/integration-tests/txn_commit-itest.cc@302 PS7, Line 302: , TestCommi > Done Thank you! http://gerrit.cloudera.org:8080/#/c/16952/7/src/kudu/integration-tests/txn_commit-itest.cc@690 PS7, Line 690: shared_ptr<KuduSessi > Yeah, calling Shutdown() from the mini server helps exercise any potential Thanks! http://gerrit.cloudera.org:8080/#/c/16952/7/src/kudu/integration-tests/txn_commit-itest.cc@754 PS7, Line 754: // TxnStatusManager. > That case is very similar to the TestCommitWhenParticipantsAreDown case, th Sure -- it seems ExternalMiniCluster-based harness fits better for that purpose. Also, if you are sure the control paths and everything else is the same, probably having just TestCommitWhenParticipantsAreDown should be good enough. http://gerrit.cloudera.org:8080/#/c/16952/7/src/kudu/transactions/txn_status_manager.cc File src/kudu/transactions/txn_status_manager.cc: http://gerrit.cloudera.org:8080/#/c/16952/7/src/kudu/transactions/txn_status_manager.cc@201 PS7, Line 201: return false; : } : > We discussed this further with Hao and agreed that it's worth exploring an Thank you! http://gerrit.cloudera.org:8080/#/c/16952/7/src/kudu/transactions/txn_status_manager.cc@201 PS7, Line 201: return false; : } : > The case we're thinking about here is if a transaction is racing with a dro Dropping a partition after commit is complete is a bit different because at least at the commit timestamp all the written data was persisted in the database. However, since right now DDL operations doesn't participate in MVCC story in Kudu, there isn't any difference -- I agree. -- To view, visit http://gerrit.cloudera.org:8080/16952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2258dded3ab3d527cb5d0abdc7d5e7deb4da15e Gerrit-Change-Number: 16952 Gerrit-PatchSet: 15 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 05 Feb 2021 06:55:47 +0000 Gerrit-HasComments: Yes
