You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "balesh2 (GitHub)" <gi...@apache.org> on 2019/03/12 21:25:11 UTC

[GitHub] [geode] balesh2 opened pull request #3296: DO NOT MERGE Geode 6515

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?

- [ ] Is your initial contribution a single, squashed commit?

- [ ] Does `gradlew build` run cleanly?

- [ ] Have you written or updated unit tests to verify your changes?

- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?

### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.


[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett commented on pull request #3296: DO NOT MERGE Geode 6515

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Yes


[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] pivotal-jbarrett commented on pull request #3296: DO NOT MERGE Geode 6515

Posted by "pivotal-jbarrett (GitHub)" <gi...@apache.org>.
Yeah, I think I have a solution for that using the iterator. 

We expected in this solution that the size was just an arbitrary limit on the times through the pool, though one very close to the entire pool. We were trying to work around a race potentially with trying to pull one off near the front of the queue while it is also being polled by as the queue drains.

[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] balesh2 closed pull request #3296: DO NOT MERGE Geode 6515

Posted by "balesh2 (GitHub)" <gi...@apache.org>.
[ pull request closed by balesh2 ]

[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] balesh2 commented on issue #3296: DO NOT MERGE Geode 6515

Posted by "balesh2 (GitHub)" <gi...@apache.org>.
Closing until we're ready to make a real PR for this

[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on pull request #3296: DO NOT MERGE Geode 6515

Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
Now that availableConnections is a concurrent queue and other threads can be concurrently adding and removing from it, it seems wrong to have the size of the queue at one point in time limit our iterations. The other complication is that we add items we took off the head back to the tail in this loop so we could actually see them more than once. We should rethink this code now that it no longer holds a lock during the iteration.

[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on issue #3296: DO NOT MERGE Geode 6515

Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
the integration tests are timing out because the following hangs: org.apache.geode.cache.client.internal.ConnectionPoolImplJUnitTest.testExecuteOp()

[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on issue #3296: DO NOT MERGE Geode 6515

Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
One of the dunit failures was a known flaky issue: https://issues.apache.org/jira/browse/GEODE-6462. This same test passed on DistributedTestOpenJDK11 and on a rerun.
So we are down to one dunit failure: ClientServerFunctionExecutionDUnitTest.testOnServerExecution_FunctionInvocationTargetException()

[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on issue #3296: DO NOT MERGE Geode 6515

Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
the dunits don't  look good. It is timing out but I have not found hanging tests (yet) in the artifacts.
This test is failing: ClientServerSessionCacheDUnitTest
canPrecreateSessionRegionBeforeStartingClient got this NPE:
org.apache.geode.test.dunit.RMIException: While invoking org.apache.geode.modules.util.ClientServerSessionCacheDUnitTest$$Lambda$50/21175845.run in VM 2 running on Host 065c55c0316b with 4 VMs
	at org.apache.geode.test.dunit.VM.executeMethodOnObject(VM.java:579)
	at org.apache.geode.test.dunit.VM.invoke(VM.java:406)
	at org.apache.geode.modules.util.ClientServerSessionCacheDUnitTest.canPrecreateSessionRegionBeforeStartingClient(ClientServerSessionCacheDUnitTest.java:124)
Caused by: org.apache.geode.cache.execute.FunctionException: java.lang.NullPointerException
	at org.apache.geode.internal.cache.execute.ServerFunctionExecutor.executeOnServer(ServerFunctionExecutor.java:157)
	at org.apache.geode.internal.cache.execute.ServerFunctionExecutor.executeFunction(ServerFunctionExecutor.java:125)
	at org.apache.geode.internal.cache.execute.AbstractExecution.execute(AbstractExecution.java:398)
	at org.apache.geode.modules.session.catalina.ClientServerSessionCache.bootstrapServers(ClientServerSessionCache.java:171)
	at org.apache.geode.modules.session.catalina.ClientServerSessionCache.initialize(ClientServerSessionCache.java:64)
	at org.apache.geode.modules.util.ClientServerSessionCacheDUnitTest.startClientSessionCache(ClientServerSessionCacheDUnitTest.java:258)
Caused by: java.lang.NullPointerException
	at org.apache.geode.cache.client.internal.pooling.ConnectionManagerImpl.borrowConnection(ConnectionManagerImpl.java:305)
	at org.apache.geode.cache.client.internal.OpExecutorImpl.executeOnServer(OpExecutorImpl.java:395)
	at org.apache.geode.cache.client.internal.OpExecutorImpl.executeOn(OpExecutorImpl.java:362)
	at org.apache.geode.cache.client.internal.PoolImpl.executeOn(PoolImpl.java:835)
	at org.apache.geode.cache.client.internal.SingleHopOperationCallable.call(SingleHopOperationCallable.java:52)

But I saw this test pass in my IDE. However in the IDE I saw these methods fail:
multipleGeodeServersCreateSessionRegion:
org.apache.geode.test.dunit.RMIException: While invoking org.apache.geode.modules.util.ClientServerSessionCacheDUnitTest$$Lambda$25/532087022.run in VM 2 running on Host 192.168.1.25 with 4 VMs
	at org.apache.geode.test.dunit.VM.executeMethodOnObject(VM.java:578)
	at org.apache.geode.test.dunit.VM.invoke(VM.java:406)
	at org.apache.geode.modules.util.ClientServerSessionCacheDUnitTest.multipleGeodeServersCreateSessionRegion(ClientServerSessionCacheDUnitTest.java:80)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.apache.geode.test.dunit.rules.AbstractDistributedRule$1.evaluate(AbstractDistributedRule.java:59)
	at org.apache.geode.test.dunit.rules.AbstractDistributedRule$1.evaluate(AbstractDistributedRule.java:59)
	at org.apache.geode.test.dunit.rules.AbstractDistributedRule$1.evaluate(AbstractDistributedRule.java:59)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
Caused by: org.apache.geode.cache.execute.FunctionException: java.lang.NullPointerException
	at org.apache.geode.internal.cache.execute.ServerFunctionExecutor.executeOnServer(ServerFunctionExecutor.java:157)
	at org.apache.geode.internal.cache.execute.ServerFunctionExecutor.executeFunction(ServerFunctionExecutor.java:125)
	at org.apache.geode.internal.cache.execute.AbstractExecution.execute(AbstractExecution.java:398)
	at org.apache.geode.modules.session.catalina.ClientServerSessionCache.bootstrapServers(ClientServerSessionCache.java:171)
	at org.apache.geode.modules.session.catalina.ClientServerSessionCache.initialize(ClientServerSessionCache.java:64)
	at org.apache.geode.modules.util.ClientServerSessionCacheDUnitTest.startClientSessionCache(ClientServerSessionCacheDUnitTest.java:258)
	at org.apache.geode.modules.util.ClientServerSessionCacheDUnitTest.lambda$2(ClientServerSessionCacheDUnitTest.java:1)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.apache.geode.test.dunit.internal.MethodInvoker.executeObject(MethodInvoker.java:123)
	at org.apache.geode.test.dunit.internal.RemoteDUnitVM.executeMethodOnObject(RemoteDUnitVM.java:69)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at sun.rmi.server.UnicastServerRef.dispatch(UnicastServerRef.java:357)
	at sun.rmi.transport.Transport$1.run(Transport.java:200)
	at sun.rmi.transport.Transport$1.run(Transport.java:197)
	at java.security.AccessController.doPrivileged(Native Method)
	at sun.rmi.transport.Transport.serviceCall(Transport.java:196)
	at sun.rmi.transport.tcp.TCPTransport.handleMessages(TCPTransport.java:568)
	at sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run0(TCPTransport.java:826)
	at sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.lambda$run$0(TCPTransport.java:683)
	at java.security.AccessController.doPrivileged(Native Method)
	at sun.rmi.transport.tcp.TCPTransport$ConnectionHandler.run(TCPTransport.java:682)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
	at org.apache.geode.cache.client.internal.pooling.ConnectionManagerImpl.borrowConnection(ConnectionManagerImpl.java:305)
	at org.apache.geode.cache.client.internal.OpExecutorImpl.executeOnServer(OpExecutorImpl.java:395)
	at org.apache.geode.cache.client.internal.OpExecutorImpl.executeOn(OpExecutorImpl.java:362)
	at org.apache.geode.cache.client.internal.PoolImpl.executeOn(PoolImpl.java:835)
	at org.apache.geode.cache.client.internal.SingleHopOperationCallable.call(SingleHopOperationCallable.java:52)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	... 3 more

cantPrecreateMismatchedSessionRegionBeforeStartingClient:
java.lang.AssertionError: 
Expecting a throwable with cause being an instance of:
 <java.lang.IllegalStateException>
but was an instance of:
 <org.apache.geode.cache.execute.FunctionException: java.lang.NullPointerException>
	at org.apache.geode.modules.util.ClientServerSessionCacheDUnitTest.cantPrecreateMismatchedSessionRegionBeforeStartingClient(ClientServerSessionCacheDUnitTest.java:170)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.apache.geode.test.dunit.rules.AbstractDistributedRule$1.evaluate(AbstractDistributedRule.java:59)
	at org.apache.geode.test.dunit.rules.AbstractDistributedRule$1.evaluate(AbstractDistributedRule.java:59)
	at org.apache.geode.test.dunit.rules.AbstractDistributedRule$1.evaluate(AbstractDistributedRule.java:59)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)



[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on issue #3296: DO NOT MERGE Geode 6515

Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
the integration tests are timing out because the following hangs: org.apache.geode.cache.client.internal.ConnectionPoolImplJUnitTest.testExecuteOp()
This test creates a mocked Op whose attempt method always throws: SocketTimeoutException.
org.apache.geode.cache.client.internal.OpExecutorImpl.execute(Op, int) has some exception handling that end up calling exchangeConnection and your new implementation of it ends up always creating a connection. So we retry and keep getting the SocketTimeoutException. The retry logic keeps adding the server that failed to a set of servers to ignore. So the old exchange would eventually throw NoAvailableServersException once all the servers were added to the attemptedServers set.

[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] dschneider-pivotal commented on pull request #3296: DO NOT MERGE Geode 6515

Posted by "dschneider-pivotal (GitHub)" <gi...@apache.org>.
havePrefillTask is used by startBackgroundPrefill which thinks it is always called while the "lock" is held.
To prevent multiple prefill tasks from getting created I think we need to make this atomic.

[ Full content available at: https://github.com/apache/geode/pull/3296 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org