Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16201 )
Change subject: KUDU-1422 resize ErrorCollector ...................................................................... Patch Set 3: (3 comments) > Patch Set 3: > > > (1 comment) > > > > just did a quick first pass at this. Is this the same as > > https://gerrit.cloudera.org/c/16200 ? > > > > If yes, can you abandon the other one? Also, for future reference, > > you can simply amend your commit, including the commit message if > > necessary and if you don't modify the Change-Id, pushing it to > > gerrit will simply update the existing review. > > Sorry for the trouble, the other one has been abandoned. No worries and thank you. http://gerrit.cloudera.org:8080/#/c/16201/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: http://gerrit.cloudera.org:8080/#/c/16201/2/java/kudu-client/src/main/java/org/apache/kudu/client/ErrorCollector.java@34 PS2, Line 34: private volatile int maxCapacity; > It's more of a memory visibility issue. maxCapacity is not final. It's init It's true that the constructor is not synchronized, but it doesn't have to be. maxCapacity is not static, so each ErrorCollector instance will have its own maxCapacity. resize() is synchronized and it can't be called until the ErrorCollector and maxCapacity are initialized by the constructor. http://gerrit.cloudera.org:8080/#/c/16201/2/java/kudu-client/src/main/java/org/apache/kudu/client/ErrorCollector.java@104 PS2, Line 104: errorQueue.poll(); > Do you guys think it'd better to actually "shrink" the size of arrayDeque w Shrinking it would mean deep copying the contents to a new queue right? I don't think it would be worth doing it, especially as we're talking about very small amounts of memory (with the default 1000 maxCapacity we're talking about 4k or 8k depending on Xmx size). http://gerrit.cloudera.org:8080/#/c/16201/2/java/kudu-client/src/main/java/org/apache/kudu/client/ErrorCollector.java@108 PS2, Line 108: maxCapacity = size; > ArrayDeque is upper unnbounded. The capacity given in the constructor is it Yep, you're right, my bad. -- To view, visit http://gerrit.cloudera.org:8080/16201 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I53731f6367aa84d6435b3bf2143e86164c8eaa1e Gerrit-Change-Number: 16201 Gerrit-PatchSet: 3 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Greg Solovyev <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 16 Jul 2020 07:59:01 +0000 Gerrit-HasComments: Yes
