alicebob / miniredis

Pure Go Redis server for Go unittests
MIT License
3.09k stars 215 forks source link

Stream XREAD last ID '$' has incorrect behavior when blocking forever #292

Closed josh-hook closed 1 year ago

josh-hook commented 2 years ago

Hi, I found an issue using the XREAD command when passing in $ as the ID. I'm using version v2.20.0.

I think this is because in the MiniRedis server it's performing the xread callback periodically and each time getting the last ID if the original XREAD command contained $. Instead I think it should be getting the last ID in the initial command and then re-using that instead of $ for future xread polls.

For example this will never return anything:

go func() {
  result, err := client.XRead(ctx, &redis.XReadArgs{
    Streams: []string{"my-stream", "$"},
    Block:   0,
  }).Result()
}()

_, err = client.XAdd(ctx, &redis.XAddArgs{
  Stream: "my-stream",
  Values: map[string]interface{"test": "data"}
}).Result()

Because when we add the new item to the stream, in MiniRedis it's then using the newest item ID in the next poll to xread.

Easiest fix looks like in cmd_stream.go it should fetch IDs before the blocking call:

    s, _ := m.db(getCtx(c).selectedDB).streamKeys[opts.streams[0]]
    opts.ids = []string{s.lastID()}
    blocking(...)

Please correct me if you think this behavior is wrong 😄

alicebob commented 2 years ago

Please correct me if you think this behavior is wrong 😄

Thanks for your issue! I will have a look soon, if no-one beats me to it :)

alicebob commented 2 years ago

That sounds logical. Would you mind making a PR? It's fine if it isn't perfect :)

josh-hook commented 2 years ago

PR here https://github.com/alicebob/miniredis/pull/293 😄

alicebob commented 2 years ago

On Thu, Sep 22, 2022 at 01:51:14AM -0700, Joshua Hook wrote:

PR here https://github.com/alicebob/miniredis/pull/293 😄

Thanks! I'll have a look soon (busy today)