Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes client shutdown problem #7836

Conversation

Projects
None yet
1 participant
@sancar
Copy link
Member

commented Mar 25, 2016

Main issue we are trying to fix is to make sure there are
no invocations left after client has shutdown.

The assert(in ClientInvocationServiceSupport) that checks
all invocations are cleared was failing.

One of the reason that an invocation is left there was
clusterService thread. In ClusterService shutdown we will
wait for executor to shutdown completely to make sure, there
is no invocation left triggered by cluster thread.

ConnectionManager close will be done before clusterService
close to make sure, cluster thread can not trigger a new
authentication while trying to shutting down.

It turns out that CleanResources Task logic is indeed not suitable
for using when client is shutting down. Because it was postponing
clearing some invocation for 1 seconds if connection is closed
in last 5 seconds. Much simpler version of clearing invocations
is implemented in place of it.

@sancar sancar self-assigned this Mar 25, 2016

@sancar sancar modified the milestones: 3.6.2, 3.6.3 Mar 25, 2016

@sancar sancar force-pushed the sancar:fix/shutdownProblemWhenConnectionUnresponsive/maint3.x branch 2 times, most recently from baa67b0 to 440ed83 Mar 25, 2016

@sancar sancar changed the title Fixes client shutdown problem when connection unresponsive Fixes client shutdown problem Mar 25, 2016

@sancar sancar force-pushed the sancar:fix/shutdownProblemWhenConnectionUnresponsive/maint3.x branch from 440ed83 to b0ece18 Mar 28, 2016

@sancar

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2016

com.hazelcast.spi.impl.operationservice.impl.Invocation_TimeoutTest.testSyncInvocationTimeoutsWhenNoResponseIsReceivedForIsStillRunningInvocation

Failing for the past 1 build (Since Failed#2231 )
Took 1 min 1 sec.
Error Message

test timed out after 60000 milliseconds
Stacktrace

org.junit.runners.model.TestTimedOutException: test timed out after 60000 milliseconds
at java.lang.Object.wait(Native Method)
at com.hazelcast.spi.impl.operationservice.impl.InvocationFuture.pollResponse(InvocationFuture.java:300)
at com.hazelcast.spi.impl.operationservice.impl.InvocationFuture.waitForResponse(InvocationFuture.java:245)
at com.hazelcast.spi.impl.operationservice.impl.InvocationFuture.get(InvocationFuture.java:222)
at com.hazelcast.spi.impl.operationservice.impl.InvocationFuture.get(InvocationFuture.java:202)
at com.hazelcast.spi.impl.operationservice.impl.Invocation_TimeoutTest.testSyncInvocationTimeoutsWhenNoResponseIsReceivedForIsStillRunningInvocation(Invocation_TimeoutTest.java:337)
@sancar

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2016

run-lab-run

@sancar

This comment has been minimized.

Copy link
Member Author

commented Mar 28, 2016

verify

@sancar sancar force-pushed the sancar:fix/shutdownProblemWhenConnectionUnresponsive/maint3.x branch 3 times, most recently from 539af70 to a22e48f Mar 28, 2016

Fixes client shutdown problem
Main issue we are trying to fix is to make sure there are
no invocations left after client has shutdown.

The assert(in ClientInvocationServiceSupport) that checks
all invocations are cleared was failing.

One of the reason that an invocation is left there was
clusterService thread. In ClusterService shutdown we will
wait for executor to shutdown completely to make sure, there
is no invocation left triggered by cluster thread.

It turns out that CleanResources Task logic is indeed not suitable
for using when client is shutting down. Because it was postponing
clearing some invocation for 1 seconds if connection is closed
in last 5 seconds. Much simpler version of clearing invocations
is implemented in place of it.

@sancar sancar force-pushed the sancar:fix/shutdownProblemWhenConnectionUnresponsive/maint3.x branch from a22e48f to 0ef1c1e Mar 29, 2016

@sancar sancar merged commit 06e739b into hazelcast:maintenance-3.x Mar 29, 2016

1 check passed

default 10616 tests run, 39 skipped, 0 failed.
Details

@sancar sancar deleted the sancar:fix/shutdownProblemWhenConnectionUnresponsive/maint3.x branch Mar 29, 2016

sancar added a commit to sancar/hazelcast that referenced this pull request Mar 30, 2016

@sancar sancar referenced this pull request Mar 30, 2016

Merged

Backport of client fixes #7867

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.