You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/07/31 19:06:17 UTC

[GitHub] [zookeeper] kaansonmezoz opened a new pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

kaansonmezoz opened a new pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-667665695


   Thanks, it looks good to me now. But there is a unit test failing (`org.apache.zookeeper.test.QuorumTest.testSessionMoved`). Although I'm not sure if it is related to your change, or something different). I'll try to re-trigger the test job


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] maoling commented on pull request #1350: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
maoling commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-626183244


   Link to [PR#1267|ZOOKEEPER-3709](https://github.com/apache/zookeeper/pull/1267/commits/ad8b17c55ae8a3421f369312643312fec44e03d7)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on a change in pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on a change in pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#discussion_r449886694



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
##########
@@ -246,18 +246,23 @@ void request(Request request) throws IOException {
             LOG.error("Throttled request sent to leader: {}. Exiting", request);
             ServiceUtils.requestSystemExit(ExitCode.UNEXPECTED_ERROR.getValue());
         }
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        // size of sessionId, cxId and type in bytes
+        int size = Long.BYTES + 2 * Integer.BYTES;
+        byte[] bytes = null;
+        if(request.request != null) {

Review comment:
       you need a space after the 'if' statement to fix the checkstyle error.
   (btw you can run checkstyle locally by: `mvn verify spotbugs:check checkstyle:check -DskipTests` )




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-667665743


   retest maven build


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] kaansonmezoz commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
kaansonmezoz commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-667573528


   > Thanks for working on this! :)
   > Sorry, I was a bit overloaded with other tasks. I'll review on the weekend or Monday.
   Yeah I know that feeling ๐Ÿ™. It's totally fine man, I was also in a similar position, that's why I hadn't contributed to this issue ๐Ÿ™ 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-667666011


   I tried it locally on my machine too. The same test runs for me on the master branch, but fails when I apply your patch. Please take a look.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on a change in pull request #1350: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on a change in pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#discussion_r436714137



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
##########
@@ -246,18 +246,23 @@ void request(Request request) throws IOException {
             LOG.error("Throttled request sent to leader: {}. Exiting", request);
             ServiceUtils.requestSystemExit(ExitCode.UNEXPECTED_ERROR.getValue());
         }
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        DataOutputStream oa = new DataOutputStream(baos);
-        oa.writeLong(request.sessionId);
-        oa.writeInt(request.cxid);
-        oa.writeInt(request.type);
-        if (request.request != null) {
+        // size of sessionId, cxId and type in bytes
+        int size = Long.BYTES + 2 * Integer.BYTES;
+        byte[] bytes = null;
+        if(request.request != null){
             request.request.rewind();
             int len = request.request.remaining();
             byte[] b = new byte[len];
             request.request.get(b);
-            request.request.rewind();
-            oa.write(b);
+            size = size + len;
+        }
+        ByteArrayOutputStream baos = new ByteArrayOutputStream(size);
+        DataOutputStream oa = new DataOutputStream(baos);
+        oa.writeLong(request.sessionId);
+        oa.writeInt(request.cxid);
+        oa.writeInt(request.type);
+        if (bytes != null) {

Review comment:
       in your code `bytes` is always `null` now, you never set it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] ruiyang00 edited a comment on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
ruiyang00 edited a comment on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-777151367


   Hello @symat, I am on it. One question regarding the zookeeper's unit test system. When we make some changes to the zookeeper-server package, do we only run the unit test under that package or do we run the `mvn test` from the zookeeper root directory? I encounter unit test failings on the current master branch before I make any changes
   
   ```
   [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 11.109 s <<< FAILURE! - in org.apache.zookeeper.ClientCnxnSocketFragilityTest
   [ERROR] testClientCnxnSocketFragility  Time elapsed: 10.919 s  <<< ERROR!
   org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss for /testClientCnxnSocketFragility
   	at org.apache.zookeeper.KeeperException.create(KeeperException.java:102)
   	at org.apache.zookeeper.KeeperException.create(KeeperException.java:54)
   	at org.apache.zookeeper.ZooKeeper.create(ZooKeeper.java:1347)
   	at org.apache.zookeeper.ClientCnxnSocketFragilityTest.testClientCnxnSocketFragility(ClientCnxnSocketFragilityTest.java:113)
   
   
   [ERROR] Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.586 s <<< FAILURE! - in org.apache.zookeeper.server.NIOServerCnxnFactoryTest
   [ERROR] testShutdownWithoutStart_SocketReleased  Time elapsed: 0.015 s  <<< ERROR!
   java.net.BindException: Address already in use
   	at java.base/sun.nio.ch.Net.bind0(Native Method)
   	at java.base/java.net.ServerSocket.bind(ServerSocket.java:396)
   	at java.base/java.net.ServerSocket.<init>(ServerSocket.java:282)
   	at java.base/java.net.ServerSocket.<init>(ServerSocket.java:173)
   	at org.apache.zookeeper.server.NIOServerCnxnFactoryTest.testShutdownWithoutStart_SocketReleased(NIOServerCnxnFactoryTest.java:70)
   
   [ERROR] Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 155.422 s <<< FAILURE! - in org.apache.zookeeper.test.FollowerResyncConcurrencyTest
   [ERROR] testResyncBySnapThenDiffAfterFollowerCrashes  Time elapsed: 47.368 s  <<< FAILURE!
   org.opentest4j.AssertionFailedError: Waiting for server up ==> expected: <true> but was: <false>
   	at org.apache.zookeeper.test.QuorumUtil.restart(QuorumUtil.java:220)
   	at org.apache.zookeeper.test.FollowerResyncConcurrencyTest.followerResyncCrashTest(FollowerResyncConcurrencyTest.java:288)
   	at org.apache.zookeeper.test.FollowerResyncConcurrencyTest.testResyncBySnapThenDiffAfterFollowerCrashes(FollowerResyncConcurrencyTest.java:168)
   
   [ERROR] Tests run: 4, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 859.563 s <<< FAILURE! - in org.apache.zookeeper.test.DisconnectedWatcherTest
   [ERROR] testManyChildWatchersAutoReset  Time elapsed: 840.987 s  <<< ERROR!
   java.util.concurrent.TimeoutException: testManyChildWatchersAutoReset() timed out after 14 minutes
   	Suppressed: java.lang.InterruptedException
   		at java.base/java.lang.Object.wait(Native Method)
   		at java.base/java.lang.Object.wait(Object.java:321)
   		at org.apache.zookeeper.ClientCnxn.submitRequest(ClientCnxn.java:1584)
   		at org.apache.zookeeper.ClientCnxn.submitRequest(ClientCnxn.java:1556)
   		at org.apache.zookeeper.ZooKeeper.create(ZooKeeper.java:1345)
   		at org.apache.zookeeper.test.DisconnectedWatcherTest.testManyChildWatchersAutoReset(DisconnectedWatcherTest.java:243)
   		at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   		at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
   		at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   		at java.base/java.lang.reflect.Method.invoke(Method.java:564)
   		at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:686)
   		at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
   		at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
   		at org.junit.jupiter.engine.extension.TimeoutInvocation.proceed(TimeoutInvocation.java:46)
   		at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
   		at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
   		at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
   		at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
   		at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
   		at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
   		at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
   		at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
   		at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
   		at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
   		at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
   		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:212)
   		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:208)
   		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:137)
   		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:71)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
   		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
   		at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
   		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
   		at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
   		at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
   		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
   		at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
   		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
   		at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
   		at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
   		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
   		at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
   		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
   		at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
   		at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
   		at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
   		at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
   		at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:248)
   		at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$5(DefaultLauncher.java:211)
   		at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:226)
   		at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:199)
   		at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:132)
   		at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:142)
   		at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:113)
   		at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
   		at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
   		at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
   		at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
   
   [INFO] Tests run: 38, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 144.584 s - in org.apache.zookeeper.ZooKeeperTest
   [INFO] Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 411.02 s - in org.apache.zookeeper.server.quorum.QuorumSSLTest
   [INFO]
   [INFO] Results:
   [INFO]
   [ERROR] Failures:
   [ERROR]   NettyServerCnxnTest.testEnableDisableThrottling_secure_random:270->runEnableDisableThrottling:379 expected: <true> but was: <false>
   [ERROR]   WatcherCleanerTest.testDeadWatcherMetrics:168 expected: <20.0> but was: <30.0>
   [ERROR]   FollowerResyncConcurrencyTest.testResyncBySnapThenDiffAfterFollowerCrashes:168->followerResyncCrashTest:288 Waiting for server up ==> expected: <true> but was: <false>
   [ERROR] Errors:
   [ERROR]   ClientCnxnSocketFragilityTest.testClientCnxnSocketFragility:113 ยป ConnectionLoss
   [ERROR]   NIOServerCnxnFactoryTest.testShutdownWithoutStart_SocketReleased:70 ยป Bind Add...
   [ERROR]   DisconnectedWatcherTest.testManyChildWatchersAutoReset ยป Timeout testManyChild...
   [INFO]
   [ERROR] Tests run: 2871, Failures: 3, Errors: 3, Skipped: 4
   [INFO]
   [INFO] ------------------------------------------------------------------------
   [INFO] BUILD FAILURE
   [INFO] ------------------------------------------------------------------------
   [INFO] Total time:  30:17 min
   [INFO] Finished at: 2021-02-10T18:49:38-08:00
   [INFO] ------------------------------------------------------------------------
   [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.22.1:test (default-test) on project zookeeper: There are test failures.
   [ERROR]
   [ERROR] Please refer to /Users/my-local-path-to/zookeeper/zookeeper-server/target/surefire-reports for the individual test results.
   [ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
   [ERROR] ExecutionException The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
   [ERROR] Command was /bin/sh -c cd /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server && /usr/local/Cellar/openjdk/15.0.1/libexec/openjdk.jdk/Contents/Home/bin/java -Xmx512m -Dtest.junit.threads=8 -Dzookeeper.junit.threadid=3 -javaagent:/Users/my-local-path-to/.m2/repository/org/jmockit/jmockit/1.48/jmockit-1.48.jar -jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire/surefirebooter2493215314041893458.jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire 2021-02-10T18-19-47_149-jvmRun3 surefire2629750460130617138tmp surefire_2511759553619092008972tmp
   [ERROR] Process Exit Code: 0
   [ERROR] Crashed tests:
   [ERROR] org.apache.zookeeper.test.QuorumZxidSyncTest
   [ERROR] ExecutionException There was an error in the forked process
   [ERROR] unable to create native thread: possibly out of memory or process/resource limits reached
   [ERROR] ExecutionException The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
   [ERROR] Command was /bin/sh -c cd /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server && /usr/local/Cellar/openjdk/15.0.1/libexec/openjdk.jdk/Contents/Home/bin/java -Xmx512m -Dtest.junit.threads=8 -Dzookeeper.junit.threadid=8 -javaagent:/Users/my-local-path-to/.m2/repository/org/jmockit/jmockit/1.48/jmockit-1.48.jar -jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire/surefirebooter7312499248512552482.jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire 2021-02-10T18-19-47_149-jvmRun8 surefire6697442250730106271tmp surefire_2258084449439190934409tmp
   [ERROR] Process Exit Code: 0
   [ERROR] Crashed tests:
   [ERROR] org.apache.zookeeper.server.quorum.EpochWriteFailureTest
   [ERROR] org.apache.maven.surefire.booter.SurefireBooterForkException: ExecutionException The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
   [ERROR] Command was /bin/sh -c cd /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server && /usr/local/Cellar/openjdk/15.0.1/libexec/openjdk.jdk/Contents/Home/bin/java -Xmx512m -Dtest.junit.threads=8 -Dzookeeper.junit.threadid=3 -javaagent:/Users/my-local-path-to/.m2/repository/org/jmockit/jmockit/1.48/jmockit-1.48.jar -jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire/surefirebooter2493215314041893458.jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire 2021-02-10T18-19-47_149-jvmRun3 surefire2629750460130617138tmp surefire_2511759553619092008972tmp
   [ERROR] Process Exit Code: 0
   [ERROR] Crashed tests:
   [ERROR] org.apache.zookeeper.test.QuorumZxidSyncTest
   [ERROR] ExecutionException There was an error in the forked process
   [ERROR] unable to create native thread: possibly out of memory or process/resource limits reached
   [ERROR] ExecutionException The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
   [ERROR] Command was /bin/sh -c cd /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server && /usr/local/Cellar/openjdk/15.0.1/libexec/openjdk.jdk/Contents/Home/bin/java -Xmx512m -Dtest.junit.threads=8 -Dzookeeper.junit.threadid=8 -javaagent:/Users/my-local-path-to/.m2/repository/org/jmockit/jmockit/1.48/jmockit-1.48.jar -jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire/surefirebooter7312499248512552482.jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire 2021-02-10T18-19-47_149-jvmRun8 surefire6697442250730106271tmp surefire_2258084449439190934409tmp
   [ERROR] Process Exit Code: 0
   [ERROR] Crashed tests:
   [ERROR] org.apache.zookeeper.server.quorum.EpochWriteFailureTest
   [ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter.awaitResultsDone(ForkStarter.java:510)
   [ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter.runSuitesForkPerTestSet(ForkStarter.java:457)
   [ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:298)
   [ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter.run(ForkStarter.java:246)
   [ERROR] 	at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeProvider(AbstractSurefireMojo.java:1183)
   [ERROR] 	at org.apache.maven.plugin.surefire.AbstractSurefireMojo.executeAfterPreconditionsChecked(AbstractSurefireMojo.java:1011)
   [ERROR] 	at org.apache.maven.plugin.surefire.AbstractSurefireMojo.execute(AbstractSurefireMojo.java:857)
   [ERROR] 	at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:137)
   [ERROR] 	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:210)
   [ERROR] 	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:156)
   [ERROR] 	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:148)
   [ERROR] 	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:117)
   [ERROR] 	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:81)
   [ERROR] 	at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:56)
   [ERROR] 	at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
   [ERROR] 	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:305)
   [ERROR] 	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:192)
   [ERROR] 	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:105)
   [ERROR] 	at org.apache.maven.cli.MavenCli.execute(MavenCli.java:957)
   [ERROR] 	at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:289)
   [ERROR] 	at org.apache.maven.cli.MavenCli.main(MavenCli.java:193)
   [ERROR] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   [ERROR] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
   [ERROR] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   [ERROR] 	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
   [ERROR] 	at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:282)
   [ERROR] 	at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:225)
   [ERROR] 	at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:406)
   [ERROR] 	at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:347)
   [ERROR] Caused by: org.apache.maven.surefire.booter.SurefireBooterForkException: The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
   [ERROR] Command was /bin/sh -c cd /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server && /usr/local/Cellar/openjdk/15.0.1/libexec/openjdk.jdk/Contents/Home/bin/java -Xmx512m -Dtest.junit.threads=8 -Dzookeeper.junit.threadid=8 -javaagent:/Users/my-local-path-to/.m2/repository/org/jmockit/jmockit/1.48/jmockit-1.48.jar -jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire/surefirebooter7312499248512552482.jar /Users/my-local-path-to/Documents/software/zookeeper/zookeeper-server/target/surefire 2021-02-10T18-19-47_149-jvmRun8 surefire6697442250730106271tmp surefire_2258084449439190934409tmp
   [ERROR] Process Exit Code: 0
   [ERROR] Crashed tests:
   [ERROR] org.apache.zookeeper.server.quorum.EpochWriteFailureTest
   [ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter.fork(ForkStarter.java:669)
   [ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter.access$600(ForkStarter.java:115)
   [ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter$2.call(ForkStarter.java:444)
   [ERROR] 	at org.apache.maven.plugin.surefire.booterclient.ForkStarter$2.call(ForkStarter.java:420)
   [ERROR] 	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
   [ERROR] 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130)
   [ERROR] 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630)
   [ERROR] 	at java.base/java.lang.Thread.run(Thread.java:832)
   [ERROR]
   [ERROR] -> [Help 1]
   [ERROR]
   [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
   [ERROR] Re-run Maven using the -X switch to enable full debug logging.
   [ERROR]
   [ERROR] For more information about the errors and possible solutions, please read the following articles:
   [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
   
   ```
   
   I pull directly from the master branch Yesterday.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] ruiyang00 edited a comment on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
ruiyang00 edited a comment on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-777151367


   Hello @symat, I am on it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] ruiyang00 commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
ruiyang00 commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-777151367


   Hello @symat,
   I am investigating it since Yesterday. I was wondering could you provide me more info about the zookeeper's unit test? I run unit test by `mvn test` through the CLI. I encounter some unit test failings even on the master branch(up-to-day) before I make any changes. 
   For instance, `org.apache.zookeeper.test.FollowerResyncConcurrencyTestI`, ` org.apache.zookeeper.server.util.RequestPathMetricsCollectorTest`, and others. 
   Yet, [https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute](url) points that we use ant. So, I am a bit confused about how the zookeeper's unit test works. Could you provide me more info? Thank you


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] kaansonmezoz commented on a change in pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
kaansonmezoz commented on a change in pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#discussion_r450416031



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
##########
@@ -246,18 +246,23 @@ void request(Request request) throws IOException {
             LOG.error("Throttled request sent to leader: {}. Exiting", request);
             ServiceUtils.requestSystemExit(ExitCode.UNEXPECTED_ERROR.getValue());
         }
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        // size of sessionId, cxId and type in bytes
+        int size = Long.BYTES + 2 * Integer.BYTES;
+        byte[] bytes = null;
+        if(request.request != null) {

Review comment:
       Oh man thanks a lot. I thought I fixed that one but it turns out there is another one out there :( 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] kaansonmezoz commented on a change in pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
kaansonmezoz commented on a change in pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#discussion_r438681179



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
##########
@@ -246,18 +246,23 @@ void request(Request request) throws IOException {
             LOG.error("Throttled request sent to leader: {}. Exiting", request);
             ServiceUtils.requestSystemExit(ExitCode.UNEXPECTED_ERROR.getValue());
         }
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        DataOutputStream oa = new DataOutputStream(baos);
-        oa.writeLong(request.sessionId);
-        oa.writeInt(request.cxid);
-        oa.writeInt(request.type);
-        if (request.request != null) {
+        // size of sessionId, cxId and type in bytes
+        int size = Long.BYTES + 2 * Integer.BYTES;
+        byte[] bytes = null;
+        if(request.request != null){
             request.request.rewind();
             int len = request.request.remaining();
             byte[] b = new byte[len];
             request.request.get(b);
-            request.request.rewind();
-            oa.write(b);
+            size = size + len;
+        }
+        ByteArrayOutputStream baos = new ByteArrayOutputStream(size);
+        DataOutputStream oa = new DataOutputStream(baos);
+        oa.writeLong(request.sessionId);
+        oa.writeInt(request.cxid);
+        oa.writeInt(request.type);
+        if (bytes != null) {

Review comment:
       I thought it was only related with this method. But I can go through all the places where ByteArrayOutputStream  is used. I will also go through other classes. Thank you for your feedback :) 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-667483392


   Thanks for working on this! :)
   Sorry, I was a bit overloaded with other tasks. I'll review on the weekend or Monday.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] kaansonmezoz commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
kaansonmezoz commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-667303663






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] kaansonmezoz edited a comment on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
kaansonmezoz edited a comment on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-667573528


   > Thanks for working on this! :)
   > Sorry, I was a bit overloaded with other tasks. I'll review on the weekend or Monday.
   
   
   Yeah I know that feeling ๐Ÿ™. It's totally fine man, I was also in a similar position, that's why I hadn't contributed to this issue ๐Ÿ™ 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] ruiyang00 commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
ruiyang00 commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-774590401


   Hello @symat,
   I am wondering that is ZOOKEEPER-3709 still open for PR? It seems like is this PR hasn't merge successfully. 
   Thanks in advance!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on pull request #1350: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-640619817


   I also changed the PR title to include the apache issue ID.
   Maybe this is what @maoling was referring by "link to".


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] ruiyang00 edited a comment on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
ruiyang00 edited a comment on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-777151367


   Hello @symat, I am on it. One more question regard the zookeeper's unit test system. We we only make changes to the zookeeper-server package, do we only run the unit test under that package or do we run the `mvn test` from the zookeeper root directory?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] kaansonmezoz removed a comment on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
kaansonmezoz removed a comment on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-667303663


   I know it's been awhile since the last time committed. Could you please review again @symat :) 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-779050647


   Unfortunately we are facing some issues with the stability of the unit tests, the community is currently working on the stabilization of the tests on the master branch before the 3.7.0 release. Asking maven to run the tests sequentially might help in the meanwhile, e.g.: `mvn test -Dsurefire.rerunFailingTestsCount=3 -fae -Pfull-build -Dsurefire-forkcount=1` 
   
   But my suggestion would be to do the rebase on your feature branch, then check which tests are failing by the CI and then you only need to re-run those few tests locally to check if they work or not... e.g.:
   `mvn test -Dtest= QuorumTest -pl zookeeper-server -Dtest.output.tofile=false`


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] ruiyang00 edited a comment on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
ruiyang00 edited a comment on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-777151367


   Hello @symat, I am on it. One question regarding the zookeeper's unit test system. When we make some changes to the zookeeper-server package, do we only run the unit test under that package or do we run the `mvn test` from the zookeeper root directory? 
   Thank you!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-775745620


   Hi @ruiyang00 ,
   
   this PR never got merged. As far as I can tell based on the comments, the PR caused some unit tests to fail, so I asked you to take a look about this failure:
   
   > Thanks, it looks good to me now. But there is a unit test failing (org.apache.zookeeper.test.QuorumTest.testSessionMoved). Although I'm not sure if it is related to your change, or something different). I'll try to re-trigger the test job
   
   > retest maven build
   
   > I tried it locally on my machine too. The same test runs for me on the master branch, but fails when I apply your patch. Please take a look.
   
   I don't see the CI jobs anymore (I guess Jenkins cleaned the logs long time ago). Anyway, a lot of changes happened on the ZooKeeper master since last August, so please rebase your PR, that will also trigger a new CI run.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] ruiyang00 edited a comment on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
ruiyang00 edited a comment on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-777151367


   Hello @symat, I am on it. One question regarding the zookeeper's unit test system. When we make some changes to the zookeeper-server package, do we only run the unit test under that package or do we run the `mvn test` from the zookeeper root directory?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] kaansonmezoz edited a comment on pull request #1350: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
kaansonmezoz edited a comment on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-626212543


   @maoling  What do you mean by "link to" ? It's already closed, not merged so that issue is still active, isn't it?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] symat commented on pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-653899206


   also you accidentally committed this file: `zookeeper-server/src/test/resources/data/invalidsnap/version-2/snapshot.83f`, please revert it.
   
   (this is a known irritating thing in ZooKeeper, some tests are changing this resource... as far as I remember maybe @maoling is already checking how to fix this)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] kaansonmezoz closed pull request #1350: ZOOKEEPER-3709: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
kaansonmezoz closed pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] kaansonmezoz commented on pull request #1350: Pre-defined the size of ByteArrayOutputStream

Posted by GitBox <gi...@apache.org>.
kaansonmezoz commented on pull request #1350:
URL: https://github.com/apache/zookeeper/pull/1350#issuecomment-626212543


   What do you mean by "link to" ? It's already closed, not merged so that issue is still active, isn't it?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org