slipenkomk/valkey
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Call signalModifiedKey after the key modification is completed (#11144)

Browse Source 
Fix `signalModifiedKey()` order, call it after key modification
completed, to ensure the state of key eventual consistency.

When a key is modified, Redis calls `signalModifiedKey` to notify other
systems, such as the watch system of transactions and the tracking
system of client side caching. However, in some commands, the
`signalModifiedKey` call happens during the key modification process
instead of after the key modification is completed. This can potentially
cause issues, as systems relying on `signalModifiedKey` may receive the
"write in flight" status of the key rather than its final state.

These commands include:
1. PFADD
2. LSET, LMOVE, LREM
3. ZPOPMIN, ZPOPMAX, BZPOPMIN, BZPOPMAX, ZMPOP, BZMPOP

Currently there is no problem with Redis, but it is better to adjust the
order of `signalModifiedKey()`, to avoid issues in future development on
Redis.

---------

Co-authored-by: zhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
This commit is contained in:
binfeng-xin 2023-11-27 11:16:41 +08:00 committed by GitHub
parent 2e854bccc6
commit 56ec1ff1ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 14 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/hyperloglog.c
View File

@ -1220,10 +1220,10 @@ void pfaddCommand(client *c) {
}
hdr = o->ptr;
if (updated) {
HLL_INVALIDATE_CACHE(hdr);
signalModifiedKey(c,c->db,c->argv[1]);
notifyKeyspaceEvent(NOTIFY_STRING,"pfadd",c->argv[1],c->db->id);
server.dirty += updated;
HLL_INVALIDATE_CACHE(hdr);
}
addReply(c, updated ? shared.cone : shared.czero);
}

26
src/t_list.c
View File

@ -631,15 +631,14 @@ void lsetCommand(client *c) {
listTypeTryConversionAppend(o,c->argv,3,3,NULL,NULL);
if (listTypeReplaceAtIndex(o,index,value)) {
addReply(c,shared.ok);
signalModifiedKey(c,c->db,c->argv[1]);
notifyKeyspaceEvent(NOTIFY_LIST,"lset",c->argv[1],c->db->id);
server.dirty++;
/* We might replace a big item with a small one or vice versa, but we've
* already handled the growing case in listTypeTryConversionAppend()
* above, so here we just need to try the conversion for shrinking. */
listTypeTryConversion(o,LIST_CONV_SHRINKING,NULL,NULL);
addReply(c,shared.ok);
signalModifiedKey(c,c->db,c->argv[1]);
notifyKeyspaceEvent(NOTIFY_LIST,"lset",c->argv[1],c->db->id);
server.dirty++;
} else {
addReplyErrorObject(c,shared.outofrangeerr);
}
@ -1086,15 +1085,14 @@ void lremCommand(client *c) {
listTypeReleaseIterator(li);
if (removed) {
signalModifiedKey(c,c->db,c->argv[1]);
notifyKeyspaceEvent(NOTIFY_LIST,"lrem",c->argv[1],c->db->id);
}
if (listTypeLength(subject) == 0) {
dbDelete(c->db,c->argv[1]);
notifyKeyspaceEvent(NOTIFY_GENERIC,"del",c->argv[1],c->db->id);
} else if (removed) {
listTypeTryConversion(subject,LIST_CONV_SHRINKING,NULL,NULL);
if (listTypeLength(subject) == 0) {
dbDelete(c->db,c->argv[1]);
notifyKeyspaceEvent(NOTIFY_GENERIC,"del",c->argv[1],c->db->id);
} else {
listTypeTryConversion(subject,LIST_CONV_SHRINKING,NULL,NULL);
}
signalModifiedKey(c,c->db,c->argv[1]);
}
addReplyLongLong(c,removed);
@ -1107,9 +1105,9 @@ void lmoveHandlePush(client *c, robj *dstkey, robj *dstobj, robj *value,
dstobj = createListListpackObject();
dbAdd(c->db,dstkey,dstobj);
}
signalModifiedKey(c,c->db,dstkey);
listTypeTryConversionAppend(dstobj,&value,0,0,NULL,NULL);
listTypePush(dstobj,value,where);
signalModifiedKey(c,c->db,dstkey);
notifyKeyspaceEvent(NOTIFY_LIST,
where == LIST_HEAD ? "lpush" : "rpush",
dstkey,

2
src/t_zset.c
View File

@ -4036,7 +4036,6 @@ void genericZpopCommand(client *c, robj **keyv, int keyc, int where, int emitkey
if (result_count == 0) { /* Do this only for the first iteration. */
char *events[2] = {"zpopmin","zpopmax"};
notifyKeyspaceEvent(NOTIFY_ZSET,events[where],key,c->db->id);
signalModifiedKey(c,c->db,key);
}
if (use_nested_array) {
@ -4055,6 +4054,7 @@ void genericZpopCommand(client *c, robj **keyv, int keyc, int where, int emitkey
dbDelete(c->db,key);
notifyKeyspaceEvent(NOTIFY_GENERIC,"del",key,c->db->id);
}
signalModifiedKey(c,c->db,key);
if (c->cmd->proc == zmpopCommand) {
/* Always replicate it as ZPOP[MIN|MAX] with COUNT option instead of ZMPOP. */