Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16193 )
Change subject: KUDU-2612 p5: RPC endpoints for txn status management ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/16193/3/src/kudu/integration-tests/ts_tablet_manager-itest.cc File src/kudu/integration-tests/ts_tablet_manager-itest.cc: http://gerrit.cloudera.org:8080/#/c/16193/3/src/kudu/integration-tests/ts_tablet_manager-itest.cc@1217 PS3, Line 1217: s(std::move(user_creds)); > What about a scenario when non-admin/non-service user credentials are used Done http://gerrit.cloudera.org:8080/#/c/16193/3/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/16193/3/src/kudu/tserver/tablet_service.cc@1267 PS3, Line 1267: // TODO(awong): once transactions are useable, add a feature flag. > What's the reason to postpone that till the time when transactions are usab Yeah I'd call it non-functional right now because the logic of the transaction status manager isn't complete (e.g. it doesn't send RPCs to the participants yet). If a future client were to try to use this feature, it would not work. So rather than having future users attempt this and see undefined behavior, I think we're better off rejecting those requests in the current state of the project. I only intend on there being one feature flag when the feature is actually usable, though that may change as the feature gets extended in the future. http://gerrit.cloudera.org:8080/#/c/16193/3/src/kudu/tserver/tserver_admin.proto File src/kudu/tserver/tserver_admin.proto: http://gerrit.cloudera.org:8080/#/c/16193/3/src/kudu/tserver/tserver_admin.proto@50 PS3, Line 50: // TODO(awong): populate this with some application-level results, like the : // actual transaction ID assigned, the next highest transaction ID available, : // etc. > Indeed. Do you think it's necessary to do so in this iteration of the patc I don't think it's necessary in this iteration of the patch. To avoid patch scope creep, I'd like to keep this patch focused on the addition of RPC endpoints, rather than application-level errors. -- To view, visit http://gerrit.cloudera.org:8080/16193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id846160724b334d2c52eb9a1bae0ffd19536bcc9 Gerrit-Change-Number: 16193 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 24 Jul 2020 00:43:57 +0000 Gerrit-HasComments: Yes
