Dan Burkert has posted comments on this change.

Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode

Patch Set 6:


File src/kudu/client/client.cc:

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.

File src/kudu/client/session-internal.cc:

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?

File src/kudu/util/monotime.h:

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-MessageType: comment
Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
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
Gerrit-HasComments: Yes

Reply via email to