chore: fix bugs in stream_family #4237
Conversation
romange
commented
Dec 2, 2024
romange
commented
- Use transaction time in streams code, similarly to how we do it in other commands. Stop using mstime() and delete unused redis code.
- Check for sequence overflow issue when passing huge sequence ids. Add a test.
| @@ -40,4 +47,25 @@ RandomPick UniquePicksGenerator::Generate() { | |||
| return max_index; | |||
| } | |||
|
|
|||
| streamConsumer* StreamCreateConsumer(streamCG* cg, string_view name, uint64_t now_ms, int flags) { | |||
There was a problem hiding this comment.
moved here from t_stream because we pass now_ms (the only differrence) instead of calling directly mstime().
In later versions of Redis/Valkey they also switched to "command time" by callng a global getter. Dragonfly uses transactional time
There was a problem hiding this comment.
there's another difference, which is that we now accept name as a string_view (which is better! but another difference :)
This also explains why below we don't need to free()
1. Use transaction time in streams code, similarly to how we do it in other commands. Stop using mstime() and delete unused redis code. 2. Check for sequence overflow issue when passing huge sequence ids. Add a test. Signed-off-by: Roman Gershman <[email protected]>
| @@ -40,4 +47,25 @@ RandomPick UniquePicksGenerator::Generate() { | |||
| return max_index; | |||
| } | |||
|
|
|||
| streamConsumer* StreamCreateConsumer(streamCG* cg, string_view name, uint64_t now_ms, int flags) { | |||
There was a problem hiding this comment.
there's another difference, which is that we now accept name as a string_view (which is better! but another difference :)
This also explains why below we don't need to free()
| @@ -563,15 +560,16 @@ int StreamAppendItem(stream* s, CmdArgList fields, streamID* added_id, streamID* | |||
| s->first_id = id; | |||
| if (added_id) | |||
| *added_id = id; | |||
| return C_OK; | |||
|
|
|||
| return 0; | |||
There was a problem hiding this comment.
C_OK/C_ERR are redis constants, EDOM/ERANGE are posix constants, so i wanted to reduce this spaghetti
