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

Returning false from a lua script causes an error #3246

Closed
eparker-tulip opened this issue Jan 22, 2025 · 1 comment
Closed

Returning false from a lua script causes an error #3246

eparker-tulip opened this issue Jan 22, 2025 · 1 comment

Comments

@eparker-tulip
Copy link

Expected Behavior

script.Run().Bool() should be able to return true or false without an error.

Current Behavior

Returning false from a lua script causes a redis: nil error also to be returned. It is not possible to return false without an error. An error occurs even if .Result() is used instead of .Bool().

Steps to Reproduce

package main

import (
	"context"
	"fmt"

	"github.com/redis/go-redis/v9"
)

func main() {
	ctx := context.Background()

	rdb := redis.NewClient(&redis.Options{
		Addr: ":6379",
	})
	_ = rdb.FlushDB(ctx).Err()

	res, err := boolTest.Run(ctx, rdb, []string{"true"}).Bool()
	fmt.Printf("Result (true): %v\n Error: %v\n", res, err)
	res, err = boolTest.Run(ctx, rdb, []string{"false"}).Bool()
	fmt.Printf("Result (false): %v\n Error: %v\n", res, err)
}

var boolTest = redis.NewScript(`
	local arg = KEYS[1]
	if arg == "false" then
		return false
	else
		return true
	end 
`)

OUTPUT

% go run .
   Result (true): true
   Error: <nil>
   Result (false): false
   Error: redis: nil

Context (Environment)

  • go-redis version v9.7.0
  • go version 1.22.2
@eparker-tulip
Copy link
Author

Ok, I was just pointed to this documentation which I hadn't found earlier: https://redis.uptrace.dev/guide/lua-scripting.html#lua-and-go-types

This confirms that returning false is converted to an error. This is an unexpected and odd design -- but at least it's documented.

@eparker-tulip eparker-tulip closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2025
eparker-tulip added a commit to tulip/oplogtoredis that referenced this issue Feb 3, 2025
…entries (#93)

Oplogtoredis runs two replicas in parallel which both write to redis,
with dedup being done by a lua script. However, both of these replicas
report the latency in time to reach redis, regardless of whether it was
the first to reach redis or not. This means that is one replica is
lagging behind, it will report larger latency values than what’s
actually reaching redis, and we don’t have a way to filter these since
they will be reported at a later time than the earlier successful write
from the other replica.

This adds a return value to the dedup script and uses it as a label for
the latency metrics.

Note: for some reason returning true/false from the lua script was
causing the performance and fault_injection tests to fail (after an
extra long time). Switching to int, returning 0 or 1, worked. Testing
showed that returning false was causing issues, and a search found
[this](https://stackoverflow.com/questions/75403292/lua-script-return-false-is-giving-error-redis-nil-during-the-evalsha)
which seems to align with this issue, where the lua script returning
false is treated as an error, which then caused OTR to retry (until the
dedupe token expired). I [opened an issue with
go-redis](redis/go-redis#3246).
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

No branches or pull requests

1 participant