Skip to content

Reload redis scripts on reconnect#143

Merged
gresrun merged 3 commits into
gresrun:masterfrom
VirtusAI:fix100-load-scripts-on-reconnect
Jul 11, 2018
Merged

Reload redis scripts on reconnect#143
gresrun merged 3 commits into
gresrun:masterfrom
VirtusAI:fix100-load-scripts-on-reconnect

Conversation

@lpfeup
Copy link
Copy Markdown
Contributor

@lpfeup lpfeup commented Jun 22, 2018

fixes #100

Copy link
Copy Markdown
Contributor

@ofirnk ofirnk left a comment

Choose a reason for hiding this comment

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

Simple and to the point 🎉

try {
loadRedisScripts();
} catch (IOException e) {
LOG.error("Failed to load redis scripts", e);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe Failed to reload Lua scripts after reconnect

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor

@ofirnk ofirnk left a comment

Choose a reason for hiding this comment

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

maybe write few words whether and how this was tested?
Anyway, It lgtm 👍

@lpfeup
Copy link
Copy Markdown
Contributor Author

lpfeup commented Jun 22, 2018

To test this, I basically initialized and started a WorkerImpl and then restarted the redis instance (redis implicitly flushes the script cache on restart).

Without this fix, WorkerImpl would terminate with a JedisNoScriptException while polling the queues after a redis restart.
With this fix, the worker keeps running after a redis restart.

EDIT: tested with redis docker v3.2.12

@ofirnk
Copy link
Copy Markdown
Contributor

ofirnk commented Jun 22, 2018

Cool, sounds good 👏

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.

If redis restarts, lua scripts aren't reloaded

4 participants