Does Kvrocks guarantee atomicity property in case of crash?

[nhancdt2602]

Hey team,

I'm posting this question to verify my understanding of the atomicity guarantees provided by Lua scripts in Kvrocks.

By atomicity, I mean the property that all write operations within a Lua script either succeed together or fail together in case Kvrocks crashes during execution.

Based on this PR: #1287, my understanding is that RocksDB's WriteBatch prevents any partial writes from being persisted. In other words, if Kvrocks crashes while executing a Lua script, none of the writes that have already been executed in the script will be visible after the server restarts.

For example, assuming the current value of A is 10:

-- Current value: A = 10

redis.call('SET', 'A', 20)

-- Kvrocks crashes here. B is never set, but A has already been updated in memory. ❌ 

redis.call('SET', 'B', 100)

After Kvrocks restarts, my expectation is that A will still be 10 because the WriteBatch was never committed.

Is my understanding correct?

[git-hulk]

@nhancdt2602 Thanks for raising this.

After Kvrocks restarts, my expectation is that A will still be 10 because the WriteBatch was never committed.

No indeed. The #1287 is only applied in transaction mode, so it's NOT crash-atomic in Lua.

[nhancdt2602]

Oh, I see. So does it make sense to introduce the similar mechanism to lua script? If so, are there any near future plan for this?

[git-hulk]

Yes for sure, that makes a lot of sense to have the crash-atomic behavior. But the following corner cases should be considered:

• EVAL/Function Call inside transaction
• Read-only script should be kept as it is
• lua-strict-key-accessing is enabled, which will run scripts in parallel

[nhancdt2602]

Thx you for verification. I'll create an issue for this soon.

[PragmaTwice]

IIRC it is guaranteed when txn-context-enabled is enabled. But I can't put my finger on it.

[git-hulk]

@nhancdt2602 Oh yes. Twice is correct. I forgot we have had this feature for a long time, even though we haven't enabled it by default yet.

[nhancdt2602]

So it means that if txn-context-enabled is true, crash-atomic behavior is assured? @git-hulk