TTL duration overflow

[tkrause]

Issue tracker is used for reporting bugs and discussing new features. Please use
stackoverflow for supporting issues.

Expected Behavior

That retrieving a TTL for a key should return a positive value.

Current Behavior

When retrieving a TTL for key with a long duration, time.Duration overflows. For example, set a key in Redis with a TTL of 116637714045176. This library effectively executes the following:

import (
	"fmt"
	"time"
)

func main() {
	i := 116637714045176
	fmt.Println(time.Second * time.Duration(i))
}

The result will be -291314h41m29.494867968s which is no where near correct as it should be 32399365012.54889 hours.

This was discovered when writing a background scan job with this library to remove TTLs that were set incorrectly previously. Because of the overflow, these longer TTL values are not represented correctly but valid in Redis.

Possible Solution

The use of time.Duration should likely be replaced with int64 instead to properly match up with Redis as the time.Duration time is in nanoseconds.

[mohitgupta07]

i can take a look to fix this one if needed.

[monkey92t]

In Go, time.Duration can represent approximately 290 years, which is sufficient in most cases. If there is a special need to set a particularly long expiration time, the EXPIREAT command can be used. There seems to be no evidence that it causes any issues.

[tkrause]

I disagree, as the TTL method returns a time.Duration which blindly assumes the value from Redis fits in time.Duration with no checks to confirm that it does.

This leads to the unexpected behavior above.

For setting a TTL maybe that's fine because in practice no one needs a TTL that long.

[monkey92t]

I disagree, as the TTL method returns a time.Duration which blindly assumes the value from Redis fits in time.Duration with no checks to confirm that it does.

This leads to the unexpected behavior above.

For setting a TTL maybe that's fine because in practice no one needs a TTL that long.

You're right, but we can't solve this problem indefinitely. For example, you used int64 to represent the TTL value, and its unit is seconds. It's a very large number, but it still has a representable range and does not mean infinity. Even so, we still face the issue of exceeding the range.

[monkey92t]

Just like using int32 to represent Unix timestamps, it does have boundary issues, but it has been used for many years. Our problem is using time.Duration to represent timeout durations. They have similar boundary issues, but 290 years is long enough, isn’t it? It doesn't seem to have caused any side effects, and time.Duration works well in Go.

[tkrause]

Redis itself uses int64 so I fail to see how that would have the same issue.

Having a redis.Duration type from int64 or embedding time.Duration would be a more correct choice.

But at the very least if you're just going to completely ignore this, it deserves a runtime check or a docs warning.