Todd Lipcon has posted comments on this change.
Change subject: Add time/watermark based garbage collection to ResultTracker
Patch Set 15:
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'
Line 502: FLAGS_remember_responses_ttl_ms = 100;
> Is it OK that the modified flags are not returned back to their default val
we already wrap all of our tests in google::FlagSaver since the KuduTest base
class has a FlagSaver member. I think some older tests that predate that
addition have their own FlagSavers - could probably remove them now.
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
Line 126: required int64 first_incomplete_seq_no = 3;
> Any concerns on backward compatibility here? If yes, does it make sense to
RequestIDPB was only added during this release cycle, so no issue with older
versions. The whole RequestIDPB itself is optional in the RPC header
Line 104: RpcContext* ctx = new RpcContext(call,
> Wouldn't it be a memory leak if the state was one of COMPLETED, IN_PROGRESS
ah, good catch... I wonder why leak sanitizer builds are not catching this.
To view, visit http://gerrit.cloudera.org:8080/3628
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>