Alexey Serbin has posted comments on this change.

Change subject: Add time/watermark based garbage collection to ResultTracker

Patch Set 15: Code-Review+1

Commit Message:

Line 9: This adds time and watermark based garbage collection to the 
Nit: the line is too long.

Line 26: multithreaded threaded test that runs GC at the same time as writes.
Nit: an extra 'threaded'
File src/kudu/rpc/

Line 502:   FLAGS_remember_responses_ttl_ms = 100;
Is it OK that the modified flags are not returned back to their default values? 
 May be, it's safer to add google::FlagSaver here?

Line 540: // This test creates a thread continuously makes requests to the 
server, some lasting longer
Nit: 'continuously makes' ---> 'continuously making'

Line 548:   FLAGS_remember_responses_ttl_ms = 10;
Ditto: consider adding google::FlagSaver to restore default values for the 
File src/kudu/rpc/rpc_header.proto:

Line 126:   required int64 first_incomplete_seq_no = 3;
Any concerns on backward compatibility here?  If yes, does it make sense to add 
corresponding information into the release notes?
File src/kudu/rpc/

Line 104:   RpcContext* ctx = new RpcContext(call,
Wouldn't it be a memory leak if the state was one of COMPLETED, IN_PROGRESS or 
STALE?  If yes, then please consider using unique_ptr<> wrapper around the 
newly allocated RpcContext instance.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <>
Gerrit-Reviewer: Alexey Serbin <>
Gerrit-Reviewer: David Ribeiro Alves <>
Gerrit-Reviewer: Jean-Daniel Cryans <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to