sui
sui copied to clipboard
[TransactionManager] lock transaction execution via `TransactionManager`
Currently it is a bit hard to lock transaction execution for all code paths we are interested in, because the scoped TxGuard object needs to be passed around. Since all transactions will go through TransactionManager
, it seems a natural place to guarantee only one transaction is in any of pending / executing / executed state.
Also, the pending_certificates
table used by TransactionManager
for recover seems to duplicate the purpose of wal
table. We should keep only one of them.
In this change,
-
TransactionManager::enqueue_to_execute()
takes on the responsibility of locking the execution of a transaction, writes the certificate topending_certificates
table, and taking shared object locks if needed. -
TransactionManager::commit()
is added, which unlock the execution of a transaction, clears its entry inpending_certificates
and shared object locks.
These two functions form a lock() / unlock() pair for certificate execution. After this change
- for consensus output,
TransactionManager
locking is active and enclosesCertTxGuard
acquire and release. - for certificates from RPC, and certificates with effects,
TransactionManager
locking is not used.
There are additional refactoring possible:
- remove CertTxGuard API for locking certificate execution.
- remove
wal
table and the associated recovery logic.
But these require integrating TransactionManager
with handle_certificate_with_effects()
first, so all code paths are covered by TransactionManager
locking. This will be a later change.
Since all transactions will go through TransactionManager
I don't know about this - I think we still want to be able to process transactions upon request (see handle_certificate in authority_server.rs).
Potentially, we could require transactions from authority_server to go through TransactionManager, but I'm not sure if that's an improvement over the current situation.
I don't know about this - I think we still want to be able to process transactions upon request (see handle_certificate in authority_server.rs).
Yes, the plan is for certificates from handle_certificate() rpc handler to go through TransactionManager as well. The advantages are:
- Make the handler cleaner and more consistent between owned object and shared object transactions. All certificates will wait on effects, instead of executing certificates directly, and hoping for shared object transaction the execution can be deduplicated.
- Client no longer has to be exposed with a variety of errors from execution, and forced to retry them.
I guess an overview about the proposed changes would be helpful. I will try to produce that.