Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19909 )
Change subject: [duplication] KUDU-3290 Support write ops to kafka with kafka client ...................................................................... Patch Set 8: (8 comments) http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/connector_manager.h File src/kudu/duplicator/connector_manager.h: http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/connector_manager.h@44 PS8, Line 44: n sensorsdata.com It is better not to expose a company name. http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/connector_manager.h@46 PS8, Line 46: <System Type, Uri> Include name? http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/connector_manager.h@87 PS8, Line 87: if (connectors_[index]->Type() == options.type && : expected_normalized_uri == exist_normalized_uri) { : return connectors_[index].get(); : } why not compare name? http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/connector_manager.h@105 PS8, Line 105: ( It is no need to CHECK connector. http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/connector_manager.h@109 PS8, Line 109: LOG(FATAL) << strings::Substitute("type: $0 is not kafka, support only kafka now.", LOG FATAL will cause thread exist. I think no need to interrupt the progress. http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/duplicator.h File src/kudu/duplicator/duplicator.h: http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/duplicator.h@80 PS8, Line 80: WorkAtRealtime Give some comments http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/duplicator.h@86 PS8, Line 86: queue_latest_opid How about renaming it to latest_opid_in_queue()? http://gerrit.cloudera.org:8080/#/c/19909/8/src/kudu/duplicator/duplicator.h@91 PS8, Line 91: set_queue_latest_opid How about renaming it to store_lastet_opid_in_queue() -- To view, visit http://gerrit.cloudera.org:8080/19909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icbd5738a61d2521b363c628f88e3699879920a49 Gerrit-Change-Number: 19909 Gerrit-PatchSet: 8 Gerrit-Owner: Yuqi Du <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) 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: Tue, 24 Oct 2023 11:20:47 +0000 Gerrit-HasComments: Yes
