Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16124 )
Change subject: KUDU-2612 p4: mechanism to create a transaction status table ...................................................................... Patch Set 9: (6 comments) http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/integration-tests/master_authz-itest.cc File src/kudu/integration-tests/master_authz-itest.cc: http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/integration-tests/master_authz-itest.cc@685 PS8, Line 685: ient_); > Does it make sense to add a scenario when such a call is performed using ot Done http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/integration-tests/txn_status_table-itest.cc File src/kudu/integration-tests/txn_status_table-itest.cc: http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/integration-tests/txn_status_table-itest.cc@152 PS8, Line 152: table > What about KuduClient::TableExists() and KuduClient::GetTableSchema()? Doe Done http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/integration-tests/txn_status_table-itest.cc@279 PS8, Line 279: > nit: creation of ? Done http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/transactions/txn_system_client.h File src/kudu/transactions/txn_system_client.h: http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/transactions/txn_system_client.h@43 PS8, Line 43: // calls to various servers. : class TxnSystemClient { : public: : static Status Create(const std::vector<std::string>& master_addrs, : std::unique_ptr<TxnSystemClient>* sys_client); : > Do you mind adding a comment on the idea behind exposing these methods as p I made it public to make it easier to interchange KuduClients. I'll make this private and add a friend class instead though. http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/transactions/txn_system_client.h@57 PS8, Line 57: (awong): when we imple > What about dropping transaction status table range? What component is goin It will likely live on the masters, since it seems easy enough to communicate most context to them in terms of what transaction ID ranges may be fully quiesced, e.g. via tserver heartbeating. For now, I'm leaving it as a TODO because I don't see it as a necessity to get the basic orchestration functionality up and running, and I'd like to get that together ASAP so more people can begin working in parallel (e.g. once orchestration is in place, we can begin thinking more concretely about how Impala will interact with us). http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/transactions/txn_system_client.cc File src/kudu/transactions/txn_system_client.cc: http://gerrit.cloudera.org:8080/#/c/16124/8/src/kudu/transactions/txn_system_client.cc@80 PS8, Line 80: 1 > Is it really intended to be a non-replicated table? For now, yes, as per the above TODO. -- To view, visit http://gerrit.cloudera.org:8080/16124 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I29b9009fa228a7749295b50516991613a28d58fa Gerrit-Change-Number: 16124 Gerrit-PatchSet: 9 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 23 Jul 2020 23:50:18 +0000 Gerrit-HasComments: Yes
