Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/18350 )
Change subject: WIP KUDU-3290 support duplication to kafka ...................................................................... Patch Set 2: (26 comments) http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/integration-tests/duplication_with_kafka-itest.cc File src/kudu/integration-tests/duplication_with_kafka-itest.cc: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/integration-tests/duplication_with_kafka-itest.cc@115 PS1, Line 115: LOG(INFO) << "kafka payload: " << msg.get_payload(); > warning: the 'empty' method should be used to check for emptiness instead o Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/integration-tests/duplication_with_kafka-itest.cc@147 PS1, Line 147: // Print the assigned partitions on assignment > warning: method 'DestroyKafka' can be made static [readability-convert-memb Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/integration-tests/duplication_with_kafka-itest.cc@208 PS1, Line 208: bool is_altering = true; > warning: parameter 'unused' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/integration-tests/duplication_with_kafka-itest.cc@254 PS1, Line 254: errors.front()->status(); > warning: method 'SimpleKuduSchema' can be made static [readability-convert- Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/integration-tests/external_mini_cluster-itest-base.cc File src/kudu/integration-tests/external_mini_cluster-itest-base.cc: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/integration-tests/external_mini_cluster-itest-base.cc@72 PS1, Line 72: } > warning: 'opts' used after it was moved [bugprone-use-after-move] Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/master/catalog_manager.cc@1997 PS1, Line 1997: // TODO(duyuqi), remove the condition check > warning: missing username/bug in TODO [google-readability-todo] Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/master/catalog_manager.cc@3283 PS1, Line 3283: } > warning: do not use 'else' after 'break' [readability-else-after-return] Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/master/catalog_manager.cc@4631 PS1, Line 4631: } > warning: Value stored to 'duplication_factor' is never read [clang-analyzer Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/rebalance/cluster_status.h File src/kudu/rebalance/cluster_status.h: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/rebalance/cluster_status.h@82 PS1, Line 82: term(term), > warning: std::move of the variable 'term' of the trivially-copyable type 'b Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/rebalance/cluster_status.h@83 PS1, Line 83: opid_index(opid_index), > warning: std::move of the variable 'opid_index' of the trivially-copyable t Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/local_tablet_writer.h File src/kudu/tablet/local_tablet_writer.h: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/local_tablet_writer.h@114 PS1, Line 114: op_state_.reset(new WriteOpState(tablet_replica_ptr.get(), &req_, nullptr)); > warning: use nullptr [modernize-use-nullptr] Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/ops/op_driver.cc File src/kudu/tablet/ops/op_driver.cc: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/ops/op_driver.cc@294 PS1, Line 294: if (op_->type() == consensus::FOLLOWER) { > warning: repeated branch in conditional chain [bugprone-branch-clone] temp http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/ops/op_driver.cc@527 PS1, Line 527: { > warning: missing username/bug in TODO [google-readability-todo] Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/ops/write_op.cc File src/kudu/tablet/ops/write_op.cc: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/ops/write_op.cc@277 PS1, Line 277: RETURN_NOT_OK(state_ptr->tablet_replica()->tablet()->ApplyRowOperations(state_ptr)); > error: unexpected namespace name 'tablet': expected expression [clang-diagn Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet.h File src/kudu/tablet/tablet.h: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet.h@192 PS1, Line 192: Status ApplyRowOperations(const std::shared_ptr<WriteOpState>& op_state_ptr) WARN_UNUSED_RESULT; > warning: non-const reference parameter 'op_state_ptr', make it const or use Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet.h@197 PS1, Line 197: const std::shared_ptr<WriteOpState>& op_state_ptr, > warning: non-const reference parameter 'op_state_ptr', make it const or use Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_bootstrap.cc@321 PS1, Line 321: const std::shared_ptr<WriteOpState>& op_state, > warning: non-const reference parameter 'op_state', make it const or use a p Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_bootstrap.cc@341 PS1, Line 341: const std::shared_ptr<WriteOpState>& op_state, > warning: non-const reference parameter 'op_state', make it const or use a p Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_replica.h File src/kudu/tablet/tablet_replica.h: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_replica.h@429 PS1, Line 429: return duplicator_->last_confirmed_opid(); > warning: return type 'const consensus::OpId' is 'const'-qualified at the to Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_replica.h@433 PS1, Line 433: void set_last_committed_opid(const consensus::OpId& last_committed_opid) { > warning: parameter 'last_committed_opid' is passed by value and only copied Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_replica.h@435 PS1, Line 435: } > warning: return type 'const consensus::OpId' is 'const'-qualified at the to Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_replica.h@548 PS1, Line 548: }; > warning: annotate this function with 'override' or (rarely) 'final' [modern Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_replica.cc@231 PS1, Line 231: // Init a ThreadPoolToken for the TabletReplica > warning: 'server_ctx' used after it was moved [bugprone-use-after-move] Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_replica.cc@251 PS1, Line 251: DnsResolver* resolver) { > warning: parameter 'duplicate_pool' is unused [misc-unused-parameters] Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tablet/tablet_replica.cc@1389 PS1, Line 1389: const deque<std::shared_ptr<WriteOpState>> ops) { > warning: the parameter 'ops' is copied for each invocation but only used as Done http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tools/ksck.h File src/kudu/tools/ksck.h: http://gerrit.cloudera.org:8080/#/c/18350/1/src/kudu/tools/ksck.h@81 PS1, Line 81: // TODO(duyuqi), In fact, !is_voter stable is duplicator, but > warning: missing username/bug in TODO [google-readability-todo] Done -- To view, visit http://gerrit.cloudera.org:8080/18350 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I842476216f09150383d06c7a6f4c53bf7f2ec018 Gerrit-Change-Number: 18350 Gerrit-PatchSet: 2 Gerrit-Owner: Yuqi Du <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Thu, 24 Mar 2022 15:06:07 +0000 Gerrit-HasComments: Yes
