Dictionary Implementation for Migrating and Importing slots #2409
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #2409 +/- ##
============================================
- Coverage 71.55% 71.39% -0.17%
============================================
Files 123 123
Lines 67453 67487 +34
============================================
- Hits 48268 48184 -84
- Misses 19185 19303 +118
🚀 New features to boost your workflow:
|
murphyjacob4
left a comment
murphyjacob4
left a comment
There was a problem hiding this comment.
Thanks! Overall change LGTM, just a few comments
Signed-off-by: Sarthak Aggarwal <[email protected]>
sarthakaggarwal97
force-pushed
the
dict-mig-imp-slots
branch
from
3ab1131 to
19cdf80
Compare
zuiderkwast
left a comment
zuiderkwast
left a comment
There was a problem hiding this comment.
Good work! I just looked quickly and put a small comment. Jacob: It's great that you're reviewing this.
Signed-off-by: Sarthak Aggarwal <[email protected]>
Signed-off-by: Sarthak Aggarwal <[email protected]>
zuiderkwast
left a comment
zuiderkwast
left a comment
There was a problem hiding this comment.
Lgtm but not a full review. I'll merge when @murphyjacob4 has approved.
src/cluster_legacy.c
Outdated
| clusterNode *getMigratingSlot(int slot) { | ||
| dictEntry *de = dictFind(server.cluster->migrating_slots_to, &slot); | ||
| clusterNode *getMigratingSlotDest(int slot) { | ||
| dictEntry *de = dictFind(server.cluster->migrating_slots_to, (void *)(long)slot); |
There was a problem hiding this comment.
Nit: For maximum portability, i guess we should ideally use intptr_t instead of long in these casts. There is no guarantee in the C standard that long is pointer-sized but intptr_t always is.
There was a problem hiding this comment.
TIL! I have made the change. thank you
There was a problem hiding this comment.
Just fyi, I referenced it from here, I am not sure if this nit would fit there too!
There was a problem hiding this comment.
I am going to update that before someone can leave a comment :)
Signed-off-by: Sarthak Aggarwal <[email protected]>
…o#2409) Resolves valkey-io#2184 --------- Signed-off-by: Sarthak Aggarwal <[email protected]>
3 participants
Resolves #2184