Dan Burkert has posted comments on this change.

Change subject: [rpc] handling ERROR_UNAVAILABLE RPC error
......................................................................


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/6640/6//COMMIT_MSG
Commit Message:

PS6, Line 16: SERVICE_UNAVAILABLE_TRY_LATER.
SERVICE_UNAVAILABLE


http://gerrit.cloudera.org:8080/#/c/6640/4/src/kudu/integration-tests/security-unknown-tsk-itest.cc
File src/kudu/integration-tests/security-unknown-tsk-itest.cc:

Line 128: // client-->tserver RPCs.
> It seems there isn't 'bool operator()' for boost:optional.  However, there 
Oh sorry about that, I thought there was an implicit conversion to bool.  SGTM.


http://gerrit.cloudera.org:8080/#/c/6640/6/src/kudu/integration-tests/security-unknown-tsk-itest.cc
File src/kudu/integration-tests/security-unknown-tsk-itest.cc:

Line 96:     FLAGS_rpc_default_keepalive_time_ms = kConnectionKeepaliveMs_;
Doesn't this need to be set on the server?


Line 175:   for (int i = 0; i < num_tablet_servers_; ++i) {
Why did you add multiple tablets?  Is there something different about the 
multiple tablet case?


Line 261:   // The client automatically retries on getting ServiceUnavailable 
from the
It looks like this retry code may be write-path specific.  There is coverage of 
the metadata paths via table create.  Perhaps it's worth trying a table scan in 
here as well to make sure that's retrying?


Line 279:   atomic<bool> importer_do_run(true);
This is pretty confusing.  It seems like this bool won't get flipped till the 
method scope ends, but for that to happen, it has to pass 500ms anyway.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87d780a4ad88c15ceaacfddf6c1b69ed053bb959
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to