Dan Burkert has posted comments on this change.

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


Patch Set 23:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3952/23/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:

Line 474:     Status s = had_errors_ ? Status::Incomplete("Some errors 
occurred")
> OK, that reasoning makes sense.
Yah, I agree IOError isn't a great fit, but the Status codes are pretty general.


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

Line 118:   // Thread-safety note: the buffer_bytes_limit_ is not supposed to 
be accessed
> I tried that, but from the reading perspective it looked worse -- there is 
I think it's important to have safety notes like this in the header so that 
callers can see these invariants without going to the implementation.  This is, 
after all, an invariant that is imposed on the caller.


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

Line 191:   std::unordered_set<internal::Batcher*> flushed_batchers_;
> That's a good question!  Besides using those for current implementation of 
Isn't the count of bytes in currently pending operations kept in the 
buffer_bytes_used_ counter?


-- 
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: 23
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