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

ISPN-6680 InterruptedException during PutAll from Hot Rod client prev… #4360

Closed
wants to merge 3 commits into from

Conversation

gustavocoding
Copy link

@gustavocoding gustavocoding commented May 19, 2016

@gustavocoding
Copy link
Author

Adding ISPN-6692 as well

@@ -30,6 +30,7 @@ object OperationStatus extends Enumeration {
val OperationTimedOut = Value(0x86) // todo: test
val NodeSuspected = Value(0x87)
val IllegalLifecycleState = Value(0x88)
val Interrupted = Value(0x89)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you actually need a different behaviour from IllegalLifecycleState, I would prefer to see a single ServerNotAvailable operation status covering both. I know sometimes these exceptions can sometimes come from another node, but it should be the core's job to retry them and/or translate them to SuspectException.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial though was to piggyback on IllegalLifecycleState, but an InterruptedException could be throw in other situations apart from server going down. For example, a long running job (script?) that gets explicitly cancelled by the user

@gustavocoding
Copy link
Author

@danberindei thoughts? TBH I was not willing to bind InterruptedException to a server stopping, do you think it should?

@danberindei
Copy link
Member

@gustavonalle If the server isn't stoppping, and a long task is being cancelled, should the client retry it? Could the server send back an IllegalLifecycleState status if it caught an InterruptedException and the server is stopping, and pass the InterruptedException unmodified otherwise?

@gustavocoding
Copy link
Author

If the server isn't stoppping, and a long task is being cancelled, should the client retry it?

IMO no, as It could have been explicitly canceled (maybe the script or task was misbehaving).
Note that in this PR it is not retrying the ExecuteOperation upon InterruptedExceptions

Could the server send back an IllegalLifecycleState status if it caught an InterruptedException and the server is stopping, and pass the InterruptedException unmodified otherwise?

It makes sense

@gustavocoding
Copy link
Author

gustavocoding commented Jun 13, 2016

@danberindei I've simplified the PR, without changing Hot Rod version, and wrapping InterruptedException inside an IllegalLifecycleException.
Reasons for this is the need to have this backported to several other branches and that we currently don't have explicitly "interruptible" operations in the server. We can review the need to expose cancelations to the client later.

@gustavocoding
Copy link
Author

rebased

@@ -226,8 +226,8 @@
IllegalStateException queryParameterNotSet(String filterConverterFactoryName);

@LogMessage(level = WARN)
@Message(value = "Server not reachable whilst closing iteration '%s'", id = 4061)
void ignoringServerUnreachable(String iterationId);
@Message(value = "Ignoring error when closing whilst closing iteration '%s'", id = 4061)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to choose between when and whilst :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed whilst closing myself and pushed to master. Trying to rebase on 8.2.x...

@danberindei
Copy link
Member

@gustavonalle after rebasing to 8.2.x I got a failure in DistTopologyChangeUnderLoadSingleOwnerTest.testRestartServerWhilePutting

java.util.concurrent.ExecutionException: org.infinispan.client.hotrod.exceptions.HotRodClientException:Request for messageId=453 returned server error (status=0x85): java.lang.NullPointerException

And this exception in the log:

19:47:43,569 DEBUG (HotRodServerWorker-119-1:[]) [HotRodExceptionHandler] Exception caught
java.lang.NullPointerException
    at org.infinispan.server.hotrod.ContextHandler.channelActive(ContextHandler.java:210) ~[infinispan-server-hotrod-8.2.3-SNAPSHOT.jar:8.2.3-SNAPSHOT]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelActive(AbstractChannelHandlerContext.java:168) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelActive(AbstractChannelHandlerContext.java:154) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.ChannelInboundHandlerAdapter.channelActive(ChannelInboundHandlerAdapter.java:64) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelActive(AbstractChannelHandlerContext.java:168) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelActive(AbstractChannelHandlerContext.java:154) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.ChannelInboundHandlerAdapter.channelActive(ChannelInboundHandlerAdapter.java:64) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelActive(AbstractChannelHandlerContext.java:168) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelActive(AbstractChannelHandlerContext.java:154) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.ChannelInboundHandlerAdapter.channelActive(ChannelInboundHandlerAdapter.java:64) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at org.infinispan.server.core.transport.StatsChannelHandler.channelActive(StatsChannelHandler.java:45) [infinispan-server-core-8.2.3-SNAPSHOT.jar:8.2.3-SNAPSHOT]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelActive(AbstractChannelHandlerContext.java:168) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelActive(AbstractChannelHandlerContext.java:154) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.DefaultChannelPipeline.fireChannelActive(DefaultChannelPipeline.java:933) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.AbstractChannel$AbstractUnsafe.register0(AbstractChannel.java:458) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.AbstractChannel$AbstractUnsafe.access$200(AbstractChannel.java:370) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.AbstractChannel$AbstractUnsafe$1.run(AbstractChannel.java:420) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:358) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:280) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.util.concurrent.SingleThreadEventExecutor$2.run(SingleThreadEventExecutor.java:112) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:137) [netty-all-4.0.36.Final.jar:4.0.36.Final]
    at java.lang.Thread.run(Thread.java:745) [?:1.8.0_77]

Closing this PR, please open a separate one for 8.2.x.

@danberindei danberindei closed this Jul 5, 2016
@gustavocoding
Copy link
Author

@danberindei #4450 for 8.2.x

@gustavocoding gustavocoding deleted the ISPN-6680 branch February 20, 2018 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants