Todd Lipcon has posted comments on this change. ( )

Change subject: KUDU-2334: Fix OutboundTransfer::TransferStarted() to work with 

Patch Set 1:


> Patch Set 1:
> the test failure seems to be infrastructure issue ?
> tablet-decoder-eval-test.0    dist-test-slave-dist-test-slave-p2pc    -2      
> None            failed to download task files: WARNING 100 
> isolateserver(1484): Adding unknown file 9a0053f2a34

yea that's odd, agree it's unrelated.
File src/kudu/rpc/rpc-test-base.h:
PS1, Line 441:       FLAGS_rpc_encrypt_loopback_connections = true;
ah, does this mean we were not successfully testing encrypted RPCs in the past 
with this test? :(
File src/kudu/rpc/
PS1, Line 740:                        // Linux, SSL enabled
hmm, is this fix related to the patch in question? I'm surprised we haven't 
seen this failure on our flaky test dashboard
PS1, Line 1223: // Helper function for TestCancellationMultiThreads.
better to document briefly what it does, rather than what it is.
PS1, Line 1224: SendRpcAndCancel
this should probably be named SendAndCancelRpcs or something, since it sends 
and cancels more than one of them
PS1, Line 1251: ASSERT_TRUE
have to use CHECK here since this isn't the main thread and gtest isn't 
PS1, Line 1256: #define BUF_SIZE (16 * 1024 * 1024)
can you use a const int defined below within the function instead?  We prefer 
'const int kBufferSize' vs a #define
PS1, Line 1258: Regression
did this test successfully repro the bug w/o the fix?
PS1, Line 1274:   unique_ptr<uint8_t[]> buf(new uint8_t[BUF_SIZE]);
              :   memset(buf.get(), 'a', BUF_SIZE);
              :   Slice slice(buf.get(), BUF_SIZE);
you could probably get away with just:

  string buf('a', kBufferSize);

and then down below pass Slice(buf)

To view, visit
To unsubscribe, visit

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7ebdcbc1ef2a3e0c5e7162f03214c232755b683
Gerrit-Change-Number: 9587
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-Comment-Date: Mon, 12 Mar 2018 22:59:10 +0000
Gerrit-HasComments: Yes

Reply via email to