Dan Burkert has posted comments on this change.
Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
Patch Set 6:
Line 400: return shared_ptr<KuduSession>();
> OK, that sounds reasonable. Do I understand correctly that you suggest to
I'm suggesting that maybe it's not needed, because the operation is infallible.
It looks like you added Status returns instead of void in a bunch of places,
but I couldn't find any places which could actually fail.
Line 30: DEFINE_int32(client_buffer_bytes_limit, 7 * 1024 * 1024,
> Why not? If we exposed them in the documentation, a user could link a bina
We don't want to impose a configuration framework on clients. Imagine if the
application used multiple libraries, all of which came with their own
configuration framework. This is a real issue in Hadoop where the HDFS client
libraries ship with a configuration framework, and it's an absolute disaster.
PS6, Line 91: waterline
> That's fun, because originally I had put 'watermark' and then figured that
Yah, I just suggest watermark to keep it consistent with the Java impl. In
reality the words are synonymous.
Line 98: messenger->ScheduleOnReactor(
> Yes, the Java implementation is different -- it does not have batchers and
I'm not quite following. It sounds like the C++ client does not have an
equivalent to the PleaseThrottle exception, and instead we block when there are
no buffers available. That makes sense. But why in that circumstance is there
a watermark? Why would you want to block earlier than necessary?
Line 133: friend MonoTime operator -(const MonoTime& t, const MonoDelta&
> I added them here because the tests I added rely on those.
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: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins