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

Method success() in WorkerImpl sometimes might not fire JOB_SUCCESS event. #28

Closed
zaremjan opened this issue Feb 15, 2013 · 7 comments
Closed
Assignees
Labels

Comments

@zaremjan
Copy link

Method in question:

 protected void success(final Job job, final Object runner, final Object result, final String curQueue) {
        this.jedis.incr(key(STAT, PROCESSED));
        this.jedis.incr(key(STAT, PROCESSED, this.name));

        this.listenerDelegate.fireEvent(JOB_SUCCESS, this, curQueue, job, runner, result, null);
    }

Lets say we have a long running job (in my case it was ~10 minutes).
Job completes, and method success() is invoked.
But this.jedis.incr(key(STAT, PROCESSED)); throws an exception:

redis.clients.jedis.exceptions.JedisConnectionException: It seems like server has closed the connection.
    at redis.clients.util.RedisInputStream.readLine(RedisInputStream.java:90)
    at redis.clients.jedis.Protocol.processStatusCodeReply(Protocol.java:85)
    at redis.clients.jedis.Protocol.process(Protocol.java:74)
    at redis.clients.jedis.Protocol.read(Protocol.java:131)
    at redis.clients.jedis.Connection.getIntegerReply(Connection.java:188)
    at redis.clients.jedis.Jedis.incr(Jedis.java:595)

And this causes this.listenerDelegate.fireEvent(JOB_SUCCESS, this, curQueue, job, runner, result, null); to be not executed. And listeners are not being notified.
Probably there is a similar issue with failure() method.

@ghost ghost assigned gresrun Feb 15, 2013
@gresrun
Copy link
Owner

gresrun commented Feb 15, 2013

Good find. I'll look into this today.

@gresrun
Copy link
Owner

gresrun commented Feb 15, 2013

@zaremjan I tried to replicate the circumstances you described in a test here:

https://gist.github.com/gresrun/4962067

However, Jedis never closed the connection. I can try to fix this but I'll need you to test it to see if I really squashed it.

gresrun pushed a commit that referenced this issue Feb 15, 2013
gresrun pushed a commit that referenced this issue Feb 15, 2013
@zaremjan
Copy link
Author

By default redis server keeps connection alive forever, and that's probably why you are not able to replicate this.
But you can change this in two ways:

  • By sending CONFIG SET timeout <numberOfSeconds> command to running redis server
  • Or by modifying redis.conf file and then running redis server.

@zaremjan
Copy link
Author

A bit improved test:
https://gist.github.com/zaremjan/4966447

gresrun pushed a commit that referenced this issue Feb 16, 2013
@gresrun
Copy link
Owner

gresrun commented Feb 16, 2013

I got your improved test passing with the latest from master. Can you confirm that this works for you?

@zaremjan
Copy link
Author

Yep. Without your fixes test fails. With them all is fine.
Thanks for fixing it so quickly.

@gresrun
Copy link
Owner

gresrun commented Feb 18, 2013

Good show.

@gresrun gresrun closed this as completed Feb 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants