-
Notifications
You must be signed in to change notification settings - Fork 876
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
fix(server): mget crash on same key get #2474
Conversation
Signed-off-by: adi_holden <adi@dragonflydb.io>
This reverts commit f69f2ec.
Signed-off-by: adi_holden <adi@dragonflydb.io>
@@ -279,6 +279,39 @@ TEST_F(StringFamilyTest, MGetCachingModeBug2276) { | |||
EXPECT_GT(bumps2, bumps1); | |||
} | |||
|
|||
TEST_F(StringFamilyTest, MGetCachingModeBug2465) { |
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! I was about to ask for a test and you pushed :)
@@ -121,7 +121,7 @@ class PrimeBumpPolicy { | |||
: bumped_items_(bumped_items) { | |||
} | |||
// returns true if key can be made less important for eviction (opposite of bump up) | |||
bool CanBumpDown(const CompactObj& obj) const { | |||
bool CanBump(const CompactObj& obj) const { |
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.
Please update comment as well
src/server/string_family.cc
Outdated
@@ -494,18 +494,18 @@ class SetResultBuilder { | |||
|
|||
SinkReplyBuilder::MGetResponse OpMGet(bool fetch_mcflag, bool fetch_mcver, const Transaction* t, | |||
EngineShard* shard) { | |||
auto keys = t->GetShardArgs(shard->shard_id()); | |||
DCHECK(!keys.empty()); | |||
auto args = t->GetShardArgs(shard->shard_id()); |
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.
Can you please still keep the 'keys' name here?
Signed-off-by: adi_holden <adi@dragonflydb.io>
Signed-off-by: adi_holden <adi@dragonflydb.io>
Signed-off-by: adi_holden <adi@dragonflydb.io>
fix: #2465
the bug: on cache mode mget bumps up items. When executing mget with the same key several times i.e mget key key we will invalidate the iterator when we bump up the item in dash table.
the fix: bump up/down items only once by using bumped_items set
This PR also reverts c225113
and updates the bumped stats and bumped_items set if the item was bumped