Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14849 )
Change subject: KUDU-2992: Avoid sending duplicated requests in catalog_manager ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/14849/4/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/14849/4/src/kudu/master/catalog_manager.cc@3490 PS4, Line 3490: , otherwise retry forever or until cancelled. There's another case below us, so maybe remove this. http://gerrit.cloudera.org:8080/#/c/14849/4/src/kudu/master/catalog_manager.cc@3499 PS4, Line 3499: tablet_id_, : target_ts_desc_->ToString() Swap these two arguments http://gerrit.cloudera.org:8080/#/c/14849/4/src/kudu/master/catalog_manager.cc@3569 PS4, Line 3569: return "AlterTable RPC for tablet " + tablet_->ToString(); Do you think it's worth including the schema version number? Or, because AlterSchema is supposed to be serialized, will the schema version number not matter, since the master will only sent a single alter request at once? http://gerrit.cloudera.org:8080/#/c/14849/4/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/14849/4/src/kudu/master/master-test.cc@1796 PS4, Line 1796: FLAGS_raft_prepare_replacement_before_eviction : ? ReplicaManagementInfoPB::PREPARE_REPLACEMENT_BEFORE_EVICTION : : ReplicaManagementInfoPB::EVICT_FIRST nit: could just use PREPARE_REPLACEMENT_BEFORE_EVICTION, since that's the mode we expect almost everyone to use. -- To view, visit http://gerrit.cloudera.org:8080/14849 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If090723821bd78e14a3c54a35cb5e471320002e9 Gerrit-Change-Number: 14849 Gerrit-PatchSet: 4 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Thu, 12 Dec 2019 01:53:09 +0000 Gerrit-HasComments: Yes
