Skip to content

prevents workers to stale when redis server went away - #243

Open
jesusch wants to merge 4 commits into
chrisboulton:masterfrom
jesusch:master
Open

prevents workers to stale when redis server went away#243
jesusch wants to merge 4 commits into
chrisboulton:masterfrom
jesusch:master

Conversation

@jesusch

@jesusch jesusch commented Apr 23, 2015

Copy link
Copy Markdown

fix for #180

unfortunatelly I have no clue, why the first access on redis()->ping() does not throw the exception
anyhow this will prevent worker from getting useless after a network failure or redis restart

@danhunsaker

Copy link
Copy Markdown
Contributor

Throwing a CredisException instead of duplicating the error handling code would probably be a good idea.

That said, it's interesting that Credis itself doesn't handle this... The reason we don't is it's supposed to do it for us.

@jesusch

jesusch commented Apr 24, 2015

Copy link
Copy Markdown
Author

I've updated the pull according to your comment

@danhunsaker

Copy link
Copy Markdown
Contributor

Looks good to me.

I still find it odd that Credis doesn't handle that on its own, though. I wonder if it's handled correctly in a more recent version of the library...

@jesusch

jesusch commented May 19, 2015

Copy link
Copy Markdown
Author

any chance that this pull requests gets merged?

@jesusch

jesusch commented Jun 16, 2015

Copy link
Copy Markdown
Author

I had to update the commit due to missing constructor message on CredisException

@danhunsaker

Copy link
Copy Markdown
Contributor

👍

@chrisboulton

Copy link
Copy Markdown
Owner

I think in theory this should also be covered by #229, which was just merged down - essentially, the next operation against Redis on an instance that's gone away will cause the worker to throw an exception (Resque_RedisException)

danhunsaker added a commit to resque/php-resque that referenced this pull request Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants