eric-maynard has posted comments on this change. Change subject: KUDU-1422 [java client] modifiable error collector capacity ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/5291/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java: Line 207: this.mutationBufferSpace = size; > still think it should be a separate setter. Done http://gerrit.cloudera.org:8080/#/c/5291/2/java/kudu-client/src/main/java/org/apache/kudu/client/ErrorCollector.java File java/kudu-client/src/main/java/org/apache/kudu/client/ErrorCollector.java: Line 43: Preconditions.checkArgument(maxCapacity > 0, "Error collector needs to be able to store at least one row error"); > why not refactor this to use setMaxCapacity? Also, looking at this, I wonde Good idea about using {{setMaxCapacity}}. I'm not sure why you would want a size of 0, but it wouldn't break anything like using a negative number would so we can make that change if you think it's best. Line 79: this.maxCapacity = maxCapacity; > if you're reducing capacity, would make sense to trim the current queue dow As it is now, you'd be prevented from adding errors after lowering the capacity, but you wouldn't throw out any existing errors that might be in the queue. I'm not sure if you'd want to dispose of errors you'd already captured, but if you'd prefer it that way let me know. -- To view, visit http://gerrit.cloudera.org:8080/5291 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iae382050eb5eaf9c169dd3ef7ed4dbf5b5f1c334 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: eric-maynard <emayn...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: eric-maynard <emayn...@cloudera.com> Gerrit-HasComments: Yes
