Adar Dembo has posted comments on this change.

Change subject: KUDU-1752 C++ client error memory should be bounded
......................................................................


Patch Set 1:

(1 comment)

> > Separately, it may be interesting to use util/mem_tracker for
 > > client-side memory limiting and tracking. Both for this and for the
 > > batcher (maybe meta cache too). The effect should be more or less
 > > the same, but it comes with preexisting infrastructure for
 > > publishing metrics.
 > 
 > That's a good idea.  I'm not sure it's realistic to implement and
 > test that in a day or two.  Do you mind if I do that separately a
 > little bit later?

Absolutely; I didn't mean to suggest you should do it now. After all, it 
wouldn't change the client interface.

http://gerrit.cloudera.org:8080/#/c/5308/1/src/kudu/client/error_collector.cc
File src/kudu/client/error_collector.cc:

Line 41: void ErrorCollector::SetMaxMemSize(size_t size_bytes) {
> A small addition: if the limit is increased, it's also necessary to make su
Your suggested approach seems fine to me, though we'll need to describe it in 
client.h too.

If you find the explanation to be too complex, I think dumbing down the 
implementation by prohibiting changes if there's even one error is fine too.


-- 
To view, visit http://gerrit.cloudera.org:8080/5308
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0b9fe3e83bab2a0b703b685ec7f3bb1db1601e5f
Gerrit-PatchSet: 1
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 <danburk...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

Reply via email to