Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16779 )
Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client ...................................................................... Patch Set 3: (3 comments) Overall looks good to me http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/client.h@444 PS3, Line 444: /// : /// An nit: A Also remove the extra line above? http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.cc File src/kudu/client/transaction-internal.cc: http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.cc@357 PS3, Line 357: DCHECK(status.IsAborted()); nit: could you add a comment adding some insight into where these errors may come from? e.g. when failing to schedule onto the reactor thread? Hoping it'd be useful for future readers to better understand how this gets called, as well as add insight into why just Aborted() and not, e.g. TimedOut() or ServiceUnavailable() or somesuch http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.cc@387 PS3, Line 387: req This gave me pause because it's scoped to be destructed at the end of the task function. But I think that's OK since we only access it when we send the RPC, which we do within this scope. -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <aser...@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-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 08 Dec 2020 23:12:20 +0000 Gerrit-HasComments: Yes
