Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/20282 )
Change subject: KUDU-3498 Scanner keeps alive in periodically ...................................................................... Patch Set 9: (4 comments) http://gerrit.cloudera.org:8080/#/c/20282/8/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/20282/8/src/kudu/client/client-test.cc@3090 PS8, Line 3090: ile (scanner.HasMoreRows()) { : SleepFor(MonoDelta::FromSeconds(1)); > Why do we need to set fault tolerant false here? Does it affect whether the Yes. In line 3065, we set fault tolerance true, it will never throw NotFound error, the assertion in line 3098 will fail. http://gerrit.cloudera.org:8080/#/c/20282/8/src/kudu/client/client-test.cc@3108 PS8, Line 3108: > Seems this test is for testing the functionality of the StopKeepAlivePeriod Done. http://gerrit.cloudera.org:8080/#/c/20282/8/src/kudu/client/client-test.cc@3121 PS8, Line 3121: > This comment does not match the actual behavior in the following code. Done http://gerrit.cloudera.org:8080/#/c/20282/8/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/20282/8/src/kudu/client/client.h@2963 PS8, Line 2963: StopKeepAliveTimer(); > nit: What about renaming it to 'StopKeepAliveTimer'? Done -- 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: 9 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: Tue, 26 Sep 2023 07:34:32 +0000 Gerrit-HasComments: Yes
