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 <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
