-
Notifications
You must be signed in to change notification settings - Fork 871
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
feat(streams): support entries_read and lag for XINFO GROUPS #1952
Conversation
src/server/rdb_test.cc
Outdated
@@ -157,9 +157,9 @@ TEST_F(RdbTest, Stream) { | |||
EXPECT_THAT(resp, ArrLen(2)); | |||
|
|||
resp = Run({"xinfo", "groups", "key:1"}); // test dereferences array of size 1 | |||
EXPECT_THAT(resp, ArrLen(8)); | |||
EXPECT_THAT(resp.GetVec(), ElementsAre("name", "g2", "consumers", IntArg(0), "pending", IntArg(0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While here, you could use the newer syntax:
EXPECT_THAT(resp, RespArray(ElementsAre(...)))
;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, thanks!
src/server/stream_family.cc
Outdated
long long entries_read; | ||
long long lag; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our (as opposed to Redis') code, we prefer strict types such as int64_t
over unspecified long long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
src/server/stream_family.cc
Outdated
@@ -884,6 +888,7 @@ OpStatus OpCreate(const OpArgs& op_args, string_view key, const CreateOpts& opts | |||
auto* shard = op_args.shard; | |||
auto& db_slice = shard->db_slice(); | |||
OpResult<PrimeIterator> res_it = db_slice.Find(op_args.db_cntx, key, OBJ_STREAM); | |||
long entries_read = SCG_INVALID_ENTRIES_READ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
src/server/stream_family_test.cc
Outdated
TEST_F(StreamFamilyTest, XInfo) { | ||
Run({"xgroup", "create", "foo", "cgroup", "0", "mkstream"}); | ||
auto resp = Run({"xinfo", "groups", "foo"}); | ||
EXPECT_THAT(resp.GetVec(), ElementsAre("name", "cgroup", "consumers", IntArg(0), "pending", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, please use EXPECT_THAT(resp, RespArray(ElementsAre(...)))
It has the advantage of not crashing if the result is not an array (that's why in many cases we used to add a check that the result is an array)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
entries_read and lag have been added to the output of
XINFO GROUPS
since Redis 7.0. Also fixed a bug that incorrectly sets the initial value ofentries_read
when a consumer group is created.fixes #1948
So now it prints: