David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1407: reassign failed tablets
......................................................................


Patch Set 10:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7440/7/src/kudu/client/scanner-internal.cc
File src/kudu/client/scanner-internal.cc:

PS7, Line 232: case tserver::TabletServerErrorPB::TABLET_FAILED: // fall-through
> Hrm, maybe, but I'm keeping this as is for now. Reasoning here was that bef
the difference seems to be that TABLET_NOT_FOUND forces a new metadata fetch 
(as well as blacklisting the server) whereas TABLET_NOT_RUNNING only 
blacklists. not sure whether it wouldn't be best to re-fetch the metadata since 
the tablet would have to be forcibly re-replicated. up to you.


http://gerrit.cloudera.org:8080/#/c/7440/10/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:

PS10, Line 618: std::
nit remove


PS10, Line 623: Use the current term to ensure the peer will be evicted.
nit: explain that otherwise this notification is ignored


http://gerrit.cloudera.org:8080/#/c/7440/10/src/kudu/tablet/tablet_replica.cc
File src/kudu/tablet/tablet_replica.cc:

PS10, Line 233: state_ == QUIESCING || state_ == SHUTDOWN
doesn't this have to include FAILED too?


http://gerrit.cloudera.org:8080/#/c/7440/10/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

PS10, Line 176: SetupErrorAndRespond(resp->mutable_error(), s, 
TabletServerErrorPB::TABLET_FAILED, context);
likely slightly cleaner to do:
    TabletServerErrorPB::Code code = TabletServerErrorPB::TABLET_NOT_RUNNING;
    if (state == tablet::FAILED) {
      s = s.CloneAndAppend((*replica)->error().ToString());
      code = TabletServerErrorPB::TABLET_FAILED;
    }
    SetupErrorAndRespond(resp->mutable_error(), s, code, context);


http://gerrit.cloudera.org:8080/#/c/7440/10/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

PS10, Line 659: s.ToString();
more informative status message


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f61585b02fbe270d215bf7f49c0d390ceee3345
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to