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

DefaultScriptExecutor always runs script twice if Exception is thrown [DATAREDIS-347]

spring-projects-issues opened this issue Sep 11, 2014 · 1 comment
in: core type: bug


Copy link

spring-projects-issues commented Sep 11, 2014

Matt Robinson opened DATAREDIS-347 and commented

The DefaultScriptExecutor seems to optimistically attempt to run the server-cached version of a script by passing the SHA1. If any exception is thrown, it assumes that this is because the script does not exist on the server and attempts to send the script and run it.
Here is the code in question:

protected <T> T eval(RedisConnection connection, RedisScript<T> script, ReturnType returnType, int numKeys,
			byte[][] keysAndArgs, RedisSerializer<T> resultSerializer) {
		Object result;
		try {
			result = connection.evalSha(script.getSha1(), returnType, numKeys, keysAndArgs);
		} catch (Exception e) {
			result = connection.eval(scriptBytes(script), returnType, numKeys, keysAndArgs);
		if (script.getResultType() == null) {
			return null;
		return deserializeResult(resultSerializer, result);

This means that ANY exception thrown by a script causes it to be run twice, and obviously any side affects of running the script (i.e. data insertions) which occur before the error are duplicated.

I believe this is likely not the intended behaviour. The DefaultScriptExecutor should verify the reason for the exception prior to re-attempting to run the script in every case.

I have seen this behaviour in 1.3.4, 1.4.0 and as of writing, the source seems to be the same

Affects: 1.4 RC1 (Evans), 1.3.4 (Dijkstra SR4)

Referenced from: pull request #108

Backported to: 1.4.1 (Evans SR1)

Copy link

spring-projects-issues commented Oct 20, 2014

Thomas Darimont commented

Made some minor tweaks, e.g. revised the scanning of the exception message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
in: core type: bug
None yet

No branches or pull requests

2 participants