Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16194 )

Change subject: KUDU-2612 p6: add coordination calls to system client
......................................................................


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16194/6/src/kudu/integration-tests/auth_token_expire-itest.cc
File src/kudu/integration-tests/auth_token_expire-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16194/6/src/kudu/integration-tests/auth_token_expire-itest.cc@470
PS6, Line 470:   // this will ensure our token expiration is
> Do you think it might be useful adding a comment explaining the destination 
> of OpenTxnStatusTable() and BeginTransaction() calls?

Done

> From the other side, looking at this test scenario again, I realized what 
> bothered me here.  Do we have a coverage for the case when first 
> txn_client->OpenTxnStatusTable() is called, then the cluster is restarted, 
> and the txn_client->BeginTransaction() call is attempted?  Will that succeed 
> as expected?

Ah, thanks for explaining the concern. I've updated this to cover that scenario.


http://gerrit.cloudera.org:8080/#/c/16194/6/src/kudu/transactions/txn_system_client.cc
File src/kudu/transactions/txn_system_client.cc:

http://gerrit.cloudera.org:8080/#/c/16194/6/src/kudu/transactions/txn_system_client.cc@177
PS6, Line 177: First, take ownership of the context.
> I meant replacing
We need to capture 'ctx' in this lambda. That approach is no good because we 
can't copy a unique_ptr<>. Capturing by reference wouldn't be good either 
because this lambda will likely not get called before 'ctx' goes out of scope 
and is destructed.

I went with ctx_raw = ctx.release() instead and removed ignore_result().



--
To view, visit http://gerrit.cloudera.org:8080/16194
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4126cb3dcf379b397f84578c2265dca3ece3d98c
Gerrit-Change-Number: 16194
Gerrit-PatchSet: 9
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: Mon, 03 Aug 2020 04:25:58 +0000
Gerrit-HasComments: Yes

Reply via email to