Repository navigation
Add GETDEL, GETEX and MSETNX string commands - #49
Conversation
Assert GETEX with no option keeps an existing TTL (not just leaves a no-TTL key alone), and that a PX value large enough to overflow time.Duration is rejected like the EX case.
Rohit-Dnath
left a comment
There was a problem hiding this comment.
Approving. Checked out and run, not just read.
I attacked the two things that usually break this exact set of commands and both hold:
GETEX with no options does not touch the TTL. This is the classic bug in GETEX implementations and it is absent here. GetExNoChange being the zero value with no default arm in the switch makes it structurally correct rather than accidentally correct, which is the version I prefer. Verified by advancing an injected clock and watching the deadline stay pinned.
Option mutual exclusivity. Pinning an exact len(args) per branch means EX 10 PERSIST, PERSIST EX 10, EX 10 PX 10 and friends all fall out as errors without a hand-written exclusivity matrix. Confirmed the key's value and TTL survive every rejected form.
MSETNX atomicity is the part I want to call out as done properly. Collecting the shards, deduping, then locking by iterating s.shards in array-index order is a deterministic global order, so reversed-key callers cannot deadlock. Holding every lock across both the existence scan and the write closes the check-then-act window rather than narrowing it. That is the correct implementation and it is not the obvious one.
Worth writing down somewhere: MSetNX is now the only place in the store holding two shard locks at once. Everything else takes one and releases it. That is what makes the ordering safe today, and it is an invariant the next multi-key command has to respect. It might be worth a comment on the function saying so, since the next person adding a multi-key write will not know to look.
Not changing anything here, but two notes. PXAT has no upper bound where EXAT does, which looks asymmetric but mirrors the existing expireAt helper and agrees with real Redis on the cases I tried. And your EX overflow guard caps at ~292 years where real Redis allows ~9.2e15 seconds, which is forced by time.Duration and is the right trade, since erroring beats silently wrapping into the past.
That last point is worth its own follow-up though: cmdSet has no such guard, so SET a v EX 10000000000 replies OK and the key instantly vanishes (TTL -2). Your GETEX rejects the same input correctly. So this PR is strictly better than the code around it. I will open that separately, no need to touch it here.
Summary
Adds the string commands
GETDEL,GETEX, andMSETNXso RAMen matches Redis for get-and-delete, get-with-expiry, and the multi-key counterpart toSETNX.Related Issue
Closes #29
Changes
GETDEL <key>— return the value and delete the key atomically; nil if missing, and a WRONGTYPE key errors without being deletedGETEX <key> [EX s | PX ms | EXAT ts | PXAT ts | PERSIST]— return the value and optionally adjust the TTL; no option leaves the TTL untouched (likeGET), a positive-but-pastEXAT/PXATdeletes the key while still returning the value, and a non-positive or overflowing expire is rejected withERR invalid expire time in 'getex' commandMSETNX <key> <val> [key val ...]— set all pairs only if none of the keys exist; all-or-nothing, returning1on write and0otherwise. It locks every involved shard in a fixed index order so concurrent multi-key writers cannot deadlockGetDel,GetEx, andMSetNXmethods ininternal/store/string.gointernal/server/cmd_string.goandcommands.gointernal/store/store_test.goandinternal/server/server_test.go(including a concurrent MSETNX race test), and documented the commands indocs/commands.mdChecklist
go test ./...)go fmt ./...)