Alexey Serbin 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>();
> I'm suggesting that maybe it's not needed, because the operation is infalli
That's true -- the method returned Status since I brought it from the previous
revision where I used a separate flusher thread, and Start() could return an
error if it could not start the flusher thread.
Thank you for pointing at that -- I removed the Status return code, so now it
Line 30: DEFINE_int32(client_buffer_bytes_limit, 7 * 1024 * 1024,
> We don't want to impose a configuration framework on clients. Imagine if t
Yes, this reasoning makes sense to me. Since David suggested to use glags for
those parameters, I though that was a good idea. Probably, David did not mean
to expose those to clients. But then the only usage for them is throughout the
tests. OK, that sounds good to me. If there is a necessity to expose those to
client applications, we can add that later.
Line 98: messenger->ScheduleOnReactor(
> I'm not quite following. It sounds like the C++ client does not have an eq
This code is about determine whether to perform flush. This is not about
determine whether to block (that when-to-block code you could find in
The waterline/watermark criterion is to determine when it's necessary to flush
the current batcher. The idea is too check how much of the buffer is taken by
the freshly submitted (not yet scheduled for flush) operations: if it's more
than the specified percentage, then flush the current batcher. Only the
current batcher has freshly submitted (not yet scheduled for flush) operations.
In my previous comment, I tried to point to the presence of batchers in the C++
client code. Probably, the confusion is due to the fact that Java
implementation does not have those. In C++ client, there might be several
batchers accommodating pending operations. However, the limit on the total
size of pending operations is enforced -- in total, all currently active
batchers cannot consume more space for their pending operations than the limit
specified via the KuduSession::SetMutationBufferSpace() method.
Does it help to resolve the confusion (if any)?
Line 133: friend MonoTime operator -(const MonoTime& t, const MonoDelta&
Actually, thank you for pointing to this. For some reason I overlooked the
fact that having small independent patches would benefit everyone.
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