Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20282 )
Change subject: KUDU-3498 Scanner keeps alive in periodically ...................................................................... Patch Set 14: (3 comments) http://gerrit.cloudera.org:8080/#/c/20282/14/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/20282/14/src/kudu/client/client-test.cc@3093 PS14, Line 3093: // Set fault tolerance false to enable the scanner expired. : ASSERT_OK(scanner.data_->mutable_configuration()->SetFaultTolerant(false)); Do we need to SetFaultTolerant every iteration in this loop? http://gerrit.cloudera.org:8080/#/c/20282/14/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/20282/14/src/kudu/client/client.h@2954 PS14, Line 2954: It has the same function with KeepAlive(). I think this sentence is not true. Could you remove it and rephrase the words about this method? http://gerrit.cloudera.org:8080/#/c/20282/14/src/kudu/client/client.h@2965 PS14, Line 2965: Of course, when scanner has read all datas, it is : /// also called. And when the scanner is destroyed, it is also called. We don't have to show the internals of function calls in the comment. Do we always need to call 'KeepAlivePeriodically' and 'StopKeepAliveTimer' in pairs? Or 'StopKeepAliveTimer' is only needed for some special cases, and what these cases are? I think this information should be included in the comment. -- To view, visit http://gerrit.cloudera.org:8080/20282 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1165d96814eb4bcd5db9b5cb60403fffc5b18c81 Gerrit-Change-Number: 20282 Gerrit-PatchSet: 14 Gerrit-Owner: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <mre...@cloudera.com> Gerrit-Reviewer: Wang Xixu <1450306...@qq.com> Gerrit-Reviewer: Yifan Zhang <chinazhangyi...@163.com> Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org> Gerrit-Reviewer: Yuqi Du <shenxingwuy...@gmail.com> Gerrit-Comment-Date: Fri, 13 Oct 2023 12:47:49 +0000 Gerrit-HasComments: Yes