spacemeshos / go-spacemesh

Go Implementation of the Spacemesh protocol full node. 💾⏰💪
https://spacemesh.io
MIT License
748 stars 211 forks source link

Fix context cancellation handling in the database code #6273

Open ivan4th opened 1 month ago

ivan4th commented 1 month ago

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.

fasmat commented 1 month ago

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