Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16044 )
Change subject: KUDU-2612 p2: introduce transaction status management ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/16044/6/src/kudu/transactions/txn_status_manager-test.cc File src/kudu/transactions/txn_status_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/16044/6/src/kudu/transactions/txn_status_manager-test.cc@281 PS6, Line 281: ReservoirSample > all_updates.size = 3 * 10, and kNumUpdatesInParallel = 20, so we're selecti Ah, it seems that was just a misunderstanding from my side: there are three elements per transaction ID, so all is well. Sorry for the noise. http://gerrit.cloudera.org:8080/#/c/16044/6/src/kudu/transactions/txn_status_manager-test.cc@328 PS6, Line 328: LOG(DFATAL) << "bad update"; > Done I still see LOG(DFATAL) instead of FAIL() here. It seems the usage of these macros should be swapped for lines 327 vs 299 ? http://gerrit.cloudera.org:8080/#/c/16044/6/src/kudu/transactions/txn_status_manager-test.cc@404 PS6, Line 404: can't > This is common all over our codebase and has virtually no grammatical diffe IIRC, in writing it's more common to avoid the contraction and use "cannot" vs "can't", while in non-formal speech "can't" is used. Not sure where to put these in-line comments in the code, but I thought it would be more in the "writing" domain. Feel free to ignore this, it's just a nit. http://gerrit.cloudera.org:8080/#/c/16044/5/src/kudu/transactions/txn_status_manager.h File src/kudu/transactions/txn_status_manager.h: http://gerrit.cloudera.org:8080/#/c/16044/5/src/kudu/transactions/txn_status_manager.h@77 PS5, Line 77: > Thanks for summarizing the discussion Alexey! I amended the TODO to be more Thank you for updates Andrew! -- To view, visit http://gerrit.cloudera.org:8080/16044 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I371bb200cf65073ae3ac7cb311ab9a0b8344a636 Gerrit-Change-Number: 16044 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 08 Jul 2020 21:14:54 +0000 Gerrit-HasComments: Yes
