Currently, HSETEX's KSN when expire-time is in the past are: 1. hset 2. hdel 3. (optional) del
and HEXPIRE: 1. hdel 2. (optional) del
We should align these, one of two options: 1. HSETEX won't send "hset" KSN in case it only deleted fields 2. HEXPIRE will send "hexpire" KSN even if expire is in the past
i vote for (1) - since the field was set and deleted in the same execution-unit, there's no point in reporting it was even set, it might confuse subscribers
here's a suggestion to correct the tests:
# hsetex sets field and ttl in the past
r hsetex myhash PX 0 FIELDS 1 f6 v6
assert_equal "pmessage * __keyspace@${db}__:myhash hdel" [$rd1 read]
assert_equal "pmessage * __keyspace@${db}__:myhash del" [$rd1 read]
r hset myhash f7 v7
assert_equal "pmessage * __keyspace@${db}__:myhash hset" [$rd1 read]
r hexpire myhash 0 FIELDS 1 f7
assert_equal "pmessage * __keyspace@${db}__:myhash hdel" [$rd1 read]
assert_equal "pmessage * __keyspace@${db}__:myhash del" [$rd1 read]
Comment From: guybe7
@oranagra @tezc can we fix it for the next release of 8.0? or is this change too "breaking"?
Comment From: tezc
@guybe7 Currently, these are aligned with EXPIRE and SET EX, right? If that's the case, I wonder if we should change it. Aligning these commands with each other will cause misalignment with EXPIRE and SET EX then.
Comment From: guybe7
@tezc IIRC @YaacovHazan mentioned that he prefers to have all the HFE commands aligned with each other
Comment From: tezc
If we were deciding from the scratch, I'd vote for option-2. Commands are executed (hexpire/hset) and then expiry is happen to be in past (hdel). It makes more sense to me. Though, I'm not sure as it will misalign with EXPIRE/SET EX. Leaving decision to you.
Comment From: oranagra
in 1 we propose that HSET will be skipping both the hset and hdel KSN, right? i.e. for a field that didn't exist before, and add an hdel ksn for an overwritten one? it's different than HEXPIRE that modifies the expiration of an existing field.
this is a new command in 8.0, right? so from my perspective, we we can still tweak it shortly after the release, unlike the HEXPIRE that's an older command.
Comment From: guybe7
i'm having second thoughts about this... if we consider HSETEX as HSET+HEXPIRE, so KSN of hset,hdel,del actually makes sense...
AFAIC we can close this issue
Comment From: oranagra
you mean if HSETEX is like an Lua script that calls HSET+HEXPIRE.. but it's not, it's a single command that does it atomically. from that perspective, the key was never really set. i tend to agree with the original claim, just wonder if we can still fix it.