----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68737/#review208705 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java Lines 1301 (patched) <https://reviews.apache.org/r/68737/#comment292860> What about "checkAndSetMaxHeapOption" ? core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java Lines 1305 (patched) <https://reviews.apache.org/r/68737/#comment292859> Nit: extract magic number to constant core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java Lines 1322-1340 (patched) <https://reviews.apache.org/r/68737/#comment292861> What does this loop do? Not easy to figure out at a cursory look. Variable naming: can't we just call the loop variable "i" instead of "ixArg" ? core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java Lines 2146-2153 (patched) <https://reviews.apache.org/r/68737/#comment292862> Consider extracting these numbers to final fields, would look better IMO. Any reason to define them as floating point literals? core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java Lines 160-162 (patched) <https://reviews.apache.org/r/68737/#comment292863> Nit: what about final int oneKB = 1 << 10; final int oneMB = 1 << 20; final int oneGB = 1 << 30; or final int oneKB = 1024; final int oneMB = 1024 * 1024; final int oneGB = 1024 * 1024 * 1024; core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java Lines 175 (patched) <https://reviews.apache.org/r/68737/#comment292864> Does this really work? ByteBuffer.allocate() returns a HeapByteBuffer instance. However, you don't save its reference, so when it's GC-time, it will be simply collected. In general this allocator code is too complicated, I would try to find a way to simplify this. - Peter Bacsko On szept. 18, 2018, 1:06 du, András Piros wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68737/ > ----------------------------------------------------------- > > (Updated szept. 18, 2018, 1:06 du) > > > Review request for oozie and Peter Bacsko. > > > Repository: oozie-git > > > Description > ------- > > OOZIE-3307 [core][oya] Limit heap usage of LauncherAM > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 0385c77b4c01a3ec7ee9147b2369db5b1548ad80 > core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java > ada7005e01c949a3800427678b95edf98953e635 > > core/src/test/java/org/apache/oozie/action/hadoop/TestBytesAndUOMConverter.java > PRE-CREATION > > core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java > 784dc96c314ca9cde3685b0d8360f9b0d73b77cb > > core/src/test/java/org/apache/oozie/action/hadoop/TestLastParameterValueExtractor.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/68737/diff/3/ > > > Testing > ------- > > New test cases / classes added: > > * `TestJavaActionExecutor` > * `TestBytesAndUOMConverter` > * `TestLastParameterValueExtractor` > > > Thanks, > > András Piros > >
