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 15: (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? Of course, SetFaultTolerant() is only need to be called once. But we can not call it out of this loop. Because we have just stopped the current tablet server(see Line 3084~3087), fault tolerance is true at this time. If set fault tolerance false, it will still read the stopped tablet server. That is not we expected. We expect it to read another replica. To make it easy, put this code 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: KeepAlivePeriodically() uses a timer to ca > I think this sentence is not true. Could you remove it and rephrase the wor Done http://gerrit.cloudera.org:8080/#/c/20282/14/src/kudu/client/client.h@2965 PS14, Line 2965: KeepAlivePeriodically and StopKeepAliveTimer are : /// called in pair. > We don't have to show the internals of function calls in the comment. 'KeepAlivePeriodically' and 'StopKeepAliveTimer' are called in pairs, because it uses a separate thread to seed keep-alive requests. So StopKeepAliveTimer is used to stop the thread when scanner has read all datas or the scanner is destroyed. -- 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: 15 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> 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: Sun, 15 Oct 2023 14:41:40 +0000 Gerrit-HasComments: Yes
