go-spacemesh icon indicating copy to clipboard operation
go-spacemesh copied to clipboard

Fix context cancellation handling in the database code

Open ivan4th opened this issue 1 year ago • 1 comments

Description

In order to detect the situation when a context used to create an SQLite transaction is canceled, the database code checks for sqlite.SQLITE_INTERRUPT error and assumes it always means context cancellation. The error is returned due to this mechanism: https://github.com/go-llsqlite/crawshaw/blob/v0.5.3/sqlitex/pool.go#L177

Another issue is that interrupting any writes may cause deadlocks: #5716 We either need to find a way to fix it or disallow using interruptible contexts for any database writes. That is, if you create a transaction with a Context passed, the transaction must be effectively read-only, e.g. any Execs should fail upon UPDATE/INSERT/DELETE/DROP statements. This may require some further consideration

Also, we need to check whether crawshaw actually frees the connections for which the context is canceled.

Additional notes:

Ideally need to have 2 connection pools internally in the sql.Database, one of which is read-only and the other one allows writes. The write-enabled pool should have just one (1) connection available. Also, only the transactions created for the read-only pool should be cancelable and the corresponding WithWriteTx(...) etc. methods should not accept a context.

The read-only pool should be used for everything except write transactions.

ivan4th avatar Aug 20 '24 11:08 ivan4th

As stated in the discussion on the #6003, maybe we can tie transaction release / rollback to the context cancellation with context.AfterFunc: https://github.com/spacemeshos/go-spacemesh/pull/6003#discussion_r1723206141

fasmat avatar Aug 20 '24 12:08 fasmat