Dan Burkert has posted comments on this change.
Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
Patch Set 7:
Incremental review, since I see a new revision has been put up. I don't think
this is going to make 0.10, so I'm going to preempt it in the meantime.
Line 400: return shared_ptr<KuduSession>();
I've traced this through, and I can't find any place that this can actually
fail. I think as it's written will be quite problematic, I can see this being
a common source of nullptr dereferences (if in fact there are cases where it
PS6, Line 1029: be
I think gflags don't apply to the client, so these will not really be
configurable. I think setting the default to be a constant and exposing
mutators on the Session instance is the best we can do.
formatting (no wrap on initial argument, and 4 spaces for initializer list).
A lot of the methods below are wrapped too much as well.
PS6, Line 91:
We typically use 'watermark' instead of 'waterline' (at least in the Java impl).
Line 98: const bool over_waterline =
This looks like it's flushing the current buffer if it's over the waterline,
but that's not exactly what the Java client does. If a write comes in over the
waterline, a PleaseThrottle exception is passed back to the user. Also, this
only ever happens if there isn't a free buffer available.
Line 133: friend MonoTime operator -(const MonoTime& t, const MonoDelta&
Maybe split the changes to MonoTime into its own review.
To view, visit http://gerrit.cloudera.org:8080/3952
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins