Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16515 )
Change subject: KUDU-2612: add txn memrowsets to tablet ...................................................................... Patch Set 4: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/16515/3/src/kudu/tablet/txn_participant-test.cc File src/kudu/tablet/txn_participant-test.cc: http://gerrit.cloudera.org:8080/#/c/16515/3/src/kudu/tablet/txn_participant-test.cc@869 PS3, Line 869: ASSERT_OK(Write(2, kAbortedTxnId)); > I'm not sure what this is referring to. Perhaps the implicit INSERT op in W Ah, I meant to add a scenario with INSERT_IGNORE for existing row, just to make sure it succeeds. But as I see the TestInsertIgnoreInTransactionMRS scenario contains that sequence, so all is well in this regard :) http://gerrit.cloudera.org:8080/#/c/16515/3/src/kudu/tablet/txn_participant-test.cc@1263 PS3, Line 1263: ()->metrics()->mrs_lookups->valu > I'm holding off on all testing of concurrent mutations of the same rows, si I see. Thank you for the explanation. http://gerrit.cloudera.org:8080/#/c/16515/4/src/kudu/util/locks.h File src/kudu/util/locks.h: http://gerrit.cloudera.org:8080/#/c/16515/4/src/kudu/util/locks.h@281 PS4, Line 281: shared_lock& operator=(shared_lock&& other) noexcept { : swap(other); : return *this; Ah, cool -- I guess this was missing to start using std::move() for shared_lock -- To view, visit http://gerrit.cloudera.org:8080/16515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1dea4031c0acb875993352d452dc4c233e35a502 Gerrit-Change-Number: 16515 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 20 Nov 2020 04:17:28 +0000 Gerrit-HasComments: Yes
