Wang Xixu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18846 )

Change subject: [Client] Add request id to trace the whole query process
......................................................................


Patch Set 19:

(7 comments)

> Patch Set 19:
>
> (1 comment)

http://gerrit.cloudera.org:8080/#/c/18846/19/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File 
java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:

http://gerrit.cloudera.org:8080/#/c/18846/19/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@401
PS19, Line 401:     if (requestId.equals("")) {
> how about using isEmpty() ?
Done


http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/client/client.cc
File src/kudu/client/client.cc:

http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/client/client.cc@1762
PS19, Line 1762:
> remove
Done


http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/client/client.cc@2319
PS19, Line 2319: data_->SetRequestId(request_id);
               :   return Status::OK();
> return data_->SetRequestId(request_id);
The definition fo SetRequestId is void.


http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:

http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/client/scan_token-internal.cc@440
PS19, Line 440:   pb.set_request_id(request_id_);
> request_id_ is empty if SetRequestId not called?
Maybe. I will set request_id_ "" default in scan_token-internal.h


http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/tserver/tablet_service.cc@2756
PS19, Line 2756:   LOG(INFO) << "request_id: " << req->request_id()
> LOG is not enough, at least add it to trace by line 2763.
Done


http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/tserver/tablet_service.cc@3038
PS19, Line 3038:   LOG(INFO) << "request_id: " << req->request_id()
> LOG is not enough, at least add it to trace by line 3074, or add a new TRAC
Done


http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/tserver/tserver.proto
File src/kudu/tserver/tserver.proto:

http://gerrit.cloudera.org:8080/#/c/18846/19/src/kudu/tserver/tserver.proto@397
PS19, Line 397:   // If not set, generate a new uuid for it.
> request_id is not generated on tserver side,  it's not needed to mention it
Done



--
To view, visit http://gerrit.cloudera.org:8080/18846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9dbae801596726fec1c85ee547128da3179345d9
Gerrit-Change-Number: 18846
Gerrit-PatchSet: 19
Gerrit-Owner: Wang Xixu <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Jian Zhang <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <[email protected]>
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: Wed, 09 Nov 2022 02:17:24 +0000
Gerrit-HasComments: Yes

Reply via email to