Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14217 )

Change subject: KUDU-2069 p1: add persistent tserver maintenance mode
......................................................................


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/14217/6/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/14217/6/src/kudu/master/sys_catalog.cc@788
PS6, Line 788:   CHECK_OK(SchemaToPB(schema_, req.mutable_schema()));
             :   KuduPartialRow row(&schema_);
             :   CHECK_OK(row.SetInt8(kSysCatalogTableColType, TSERVER_STATE));
             :   CHECK_OK(row.SetStringNoCopy(kSysCatalogTableColId, 
tserver_id));
RETURN_NOT_OK for these?


http://gerrit.cloudera.org:8080/#/c/14217/6/src/kudu/master/ts_manager.h
File src/kudu/master/ts_manager.h:

http://gerrit.cloudera.org:8080/#/c/14217/6/src/kudu/master/ts_manager.h@94
PS6, Line 94:   Status SetTServerState(const std::string& ts_uuid,
Doc.


http://gerrit.cloudera.org:8080/#/c/14217/6/src/kudu/master/ts_manager.h@102
PS6, Line 102:   Status ReloadTServerStates(SysCatalogTable* sys_catalog);
Doc.


http://gerrit.cloudera.org:8080/#/c/14217/6/src/kudu/master/ts_manager.h@122
PS6, Line 122:   std::unordered_map<std::string, TServerStatePB> 
ts_state_by_uuid_;
We talked about this in person last week and I thought you were going to add 
TServerStatePB to TSDescriptor and reuse  servers_by_id_ (after modifying 
TSDescriptor to support "persisted but not yet registered" descriptors). What 
changed your mind?


http://gerrit.cloudera.org:8080/#/c/14217/4/src/kudu/master/ts_state.h
File src/kudu/master/ts_state.h:

http://gerrit.cloudera.org:8080/#/c/14217/4/src/kudu/master/ts_state.h@27
PS4, Line 27:
            :
            :
            :
> You previously raised concerns about mixing on-disk enums with non-on-disk
Yeah, I don't view the elision of certain special states from the overall set 
of states as a leak in the abstraction. But edge-triggered (ENTER_state) vs. 
level-triggered (IN_state) feels meaningful to me, which is why I advocated for 
different enums for RPC and persistence.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib669b43b3cee171c4c7dbd54041e29c30cb9f767
Gerrit-Change-Number: 14217
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Greg Solovyev <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 16 Sep 2019 23:04:39 +0000
Gerrit-HasComments: Yes

Reply via email to