Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24375 )

Change subject: IMPALA-14838: Count open deferred RPC intervals
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/24375/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24375/4//COMMIT_MSG@20
PS4, Line 20: Add a data-stream regression test that keeps the receiver reader
            : stopped, waits until an RPC is deferred, and verifies that the 
live
            : profile reports deferred RPC time before cancellation drains the
            : queue.
nit: Can you also add something similar to the test in comment?


http://gerrit.cloudera.org:8080/#/c/24375/4//COMMIT_MSG@26
PS4, Line 26: - git diff --check
            : - ninja java
            : - data-stream-test with filter:
            :   DataStreamTest.TotalHasDeferredRpcsTimeIncludesOpenInterval
            : - data-stream-test
nit: probably this much info is not needed, it is enough to mention that you 
ran data-stream-test


http://gerrit.cloudera.org:8080/#/c/24375/4/be/src/runtime/data-stream-test.cc
File be/src/runtime/data-stream-test.cc:

http://gerrit.cloudera.org:8080/#/c/24375/4/be/src/runtime/data-stream-test.cc@834
PS4, Line 834:   ASSERT_GT(total_deferred_rpcs->value(), 0);
Shouldn't the assert check this earlier, after the first 5 sec wait?


http://gerrit.cloudera.org:8080/#/c/24375/4/be/src/runtime/krpc-data-stream-recvr.cc
File be/src/runtime/krpc-data-stream-recvr.cc:

http://gerrit.cloudera.org:8080/#/c/24375/4/be/src/runtime/krpc-data-stream-recvr.cc@376
PS4, Line 376:   if (deferred_rpcs_.empty()) {
             :     recvr_->total_has_deferred_rpcs_timer_->Stop();
             :   }
nit: fits to 1 line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efe8016b268e3707a2b7b99c488de69ccdad8e6
Gerrit-Change-Number: 24375
Gerrit-PatchSet: 4
Gerrit-Owner: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Wed, 03 Jun 2026 15:02:03 +0000
Gerrit-HasComments: Yes

Reply via email to