Alexey Serbin has posted comments on this change.

Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode
......................................................................


Patch Set 22:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3952/22/src/kudu/client/session-internal.cc
File src/kudu/client/session-internal.cc:

PS22, Line 191:   Synchronizer s;
              :   KuduStatusMemberCallback<Synchronizer> ksmcb(&s, 
&Synchronizer::StatusCB);
              :   NextBatcher(session, BATCHER_WATERMARK_NON_EMPTY, &ksmcb, 
BLOCK);
              :   RETURN_NOT_OK(s.Wait());
> I wasn't asking about NextBatcher()'s wait; I was asking about s.Wait(). To
OK, I see the point now.

Right -- waiting on total_bytes_used_ should be more than enough given the fact 
the flush of the current batcher has been initiated.

I was concerned with collecting the exact error, if any, from the current 
batcher and missed the point that error_collector_ would account for that error 
as well.

And yes, if we separate two responsibilities of the NextBatch() method, then 
all that inconsistency will go away, so the next batcher would be allocated in 
Apply()/ApplyAsync() and FlushAsync() would not report on problems due to max 
number of batchers.


-- 
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: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to