Wang Xixu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19571 )

Change subject: KUDU-3452 Create tablet without enough healthy tservers
......................................................................


Patch Set 3:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/19571/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19571/2//COMMIT_MSG@7
PS2, Line 7: KUDU-3452 Create tablet without enough healthy tservers
> add a tag like: [master] ?
Done


http://gerrit.cloudera.org:8080/#/c/19571/2//COMMIT_MSG@21
PS2, Line 21: create tablet without enough
            : healthy tservers can success. And its status is under
            : replicating
> We don't need the number of healthy tserver greater or equal 'replication_f
ok


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5930
PS2, Line 5930:     const int alive_num_ts = ts_descs.size();
> Why move the statement from outer to inner loop? It used healthy tablet ser
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5931
PS2, Line 5931:     PlacementPolicy policy(std::move(ts_descs), &rng_);
> Whether the lock can put into {   TableMetadataLock table_guard(tablet->tab
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/master/catalog_manager.cc@5932
PS2, Line 5932:     for (auto& tablet : deferred.needs_create_rpc) {
              :       int nreplicas = 0;
              :       // Get replica factor.
              :       {
              :         TableMetadataLock table_guard(tablet->table().get(), 
LockMode::READ);
              :        
> At this, I have different opinion about this which may relation to lines fr
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9182
PS2, Line 9182: .
> 'catalog_manager_check_ts_count_for_create_table''s default value is true.
Yes


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9203
PS2, Line 9203:   // Sleep for 2s to make the state of one tab
> replacing the 'SleepFor' by checking 'ksck not OK' ?
Ksck can not make master know that tablet server is offline. Tablet server will 
report its status through heart beat every tserver_unresponsive_timeout_ms time.


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9209
PS2, Line 9209:   workload2.set_table_name(kTableName2);
> IMO, it's better start a write thread and write some records to prove the t
Done


http://gerrit.cloudera.org:8080/#/c/19571/2/src/kudu/tools/kudu-tool-test.cc@9213
PS2, Line 9213: 2.rows_inserted(
> replacing the 'SleepFor' by checking 'ksck not OK' ?
Ksck can not make master know that tablet server is offline. Tablet server will 
report its status through heart beat every tserver_unresponsive_timeout_ms time.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I742ba1ff770f5c8b1be5800334c29bec96e195c6
Gerrit-Change-Number: 19571
Gerrit-PatchSet: 3
Gerrit-Owner: Wang Xixu <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <[email protected]>
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: Yuqi Du <[email protected]>
Gerrit-Comment-Date: Mon, 13 Mar 2023 07:44:02 +0000
Gerrit-HasComments: Yes

Reply via email to