Back to Stackexchange Redis

SER301: transaction may be replaceable by a single atomic operation

docs/rules/SER301.md

3.1.133.9 KB
Original Source

SER301: transaction may be replaceable by a single atomic operation

A transaction implementing compare-and-set can usually be replaced by the equivalent conditional command on a server that supports it.

c#
// flagged
var tran = db.CreateTransaction();
tran.AddCondition(Condition.StringEqual(key, token));
_ = tran.KeyDeleteAsync(key);
await tran.ExecuteAsync();

// suggested
await db.StringDeleteAsync(key, ValueCondition.Equal(token));

That example is the canonical lock-release; LockRelease does the same thing for you.

Server version

These commands (SET IFEQ/IFNE, DELIFEQ) arrived in Redis 8.4 - see Compare-And-Swap / Compare-And-Delete. This is the whole reason the rule has its own ID rather than sharing SER300: an analyzer cannot see which server you will connect to, so if you target an older server you want to silence this one while keeping SER300, and a shared ID would not let you.

The library's own compatibility fallbacks suppress this rule rather than being rewritten, for the same reason.

Declaring your server version

Rather than silencing the rule outright, you can tell it what you are running, and it will only suggest things your server can actually do:

xml
<PropertyGroup>
  <RedisMinServerVersion>7.4</RedisMinServerVersion>
</PropertyGroup>

or equivalently in .editorconfig / .globalconfig, which takes precedence:

ini
redis.min_server_version = 7.4

Major.minor is what is read; a patch component is accepted and ignored. Unset means show everything - a suggestion you cannot use yet is still worth knowing about, and defaulting to silence would hide the rule from exactly the people who have not thought about server versions. A value that cannot be parsed is treated as unset, so a typo cannot silently hide suggestions.

This affects only the version-gated rules. SER300 is unaffected however low you set it, because the conditional argument forms it suggests are as old as the commands themselves.

Why this is worth changing

AddCondition is WATCH-based: two round-trips, and it can abort under contention, so correct code needs a retry loop. The conditional command is one round-trip evaluated atomically on the server, with no abort.

What changes when you apply it

  • The result means something different. tran.Execute() returns "the conditions held and the commands ran"; the single command returns its own result.
  • The queued Task<T> goes away, so rewire anything that awaited it.
  • CommandFlags must be carried over verbatim, including FireAndForget.

Cases that are deliberately not flagged

Only one condition guarding one queued command on the same key expression is flagged. Cross-key compare-and-set genuinely needs the transaction (or Lua), and there is no server-side compare-and-set for hash fields or list indices, so HashEqual and ListIndexEqual are left alone.

Plus everything under when these rules stay quiet.

Guidance, not a verdict

This rule is a heuristic. It reads your source text - it cannot see your keys, your server, or what you know about the code - so it is deliberately conservative and stays quiet wherever it is unsure. Everything it flags still works, and will keep working: this is a suggestion, not a defect report.

That conservatism is meant to make a false positive rare, not impossible. If you think the rule has flagged something it should not have, please report it, including the transaction as written. A rule that fires on correct code is a bug in the rule - and one that reaches every consumer of the package - so it is worth fixing rather than quietly suppressing.

Suppressing

Reported as a warning, so TreatWarningsAsErrors builds fail until you act on it or turn it down. To silence:

xml
<NoWarn>$(NoWarn);SER301</NoWarn>

or locally:

c#
#pragma warning disable SER301