MatrixAI / js-db

Key-Value DB for TypeScript and JavaScript Applications
https://polykey.com
Apache License 2.0
5 stars 0 forks source link

Fix committing or rollbacking dangling transactions #53

Closed CMCDragonkai closed 1 year ago

CMCDragonkai commented 1 year ago

Description

In the EFS, we came across a problem where there is a dangling transaction that is still pending. It may be neither committed, nor rollbacked, or it may be in the process of committing or rollbacking.

Either way, when the DB.stop() is called, it proceeds to rollback all existing transactions in the transaction reference set.

Ideally, all transactions should be committed or rollbacked before the await db.stop() is called. However if there are dangling transactions there should be rollbacked if their status is neither committed or rollbacked, but if they have a particular status, then the DB.stop should be waiting for those pending statuses to finish.

Here we introduce a lock to be used between DBTransaction.destroy, DBTransaction.commit and DBTransaction.rollback. The basic idea is for destroy to wait on a commit or rollback to finish. This allows one to call await transaction.destroy() while the committing or rollbacking is occurring.

However at the same time, if a transaction is pending to commit, but hasn't started committing. It's possible for the DB.stop() to already start rollbacking. If this occurs, during the resource release, the transaction will attempt to commit, in that case, an exception still occurs because the transaction is already rollbacked. This is a legitimate exception.

Issues Fixed

Tasks

Final checklist

ghost commented 1 year ago
👇 Click on the image for a new way to code review - Make big changes easier — review code in small groups of related files - Know where to start — see the whole change at a glance - Take a code tour — explore the change with an interactive tour - Make comments and review — all fully sync’ed with github [Try it now!](https://app.codesee.io/r/reviews?pr=53&src=https%3A%2F%2Fgithub.com%2FMatrixAI%2Fjs-db)

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

CMCDragonkai commented 1 year ago

@tegefaulkes the exception in EFS may still occur simply because the EFS could stop the database before the committing or rollbacking has even started.

In that case the dangling transaction attempts to release which will either commit or rollback. But this will cause an exception. This is unavoidable UNLESS we were to make is that committ or rollback is a noop if it is already committing or rollbacking.

CMCDragonkai commented 1 year ago

The DBTransaction.committed and DBTransaction.rollbacked is a bit of a misnomer. These 2 actually mean that it has started committing or rollbacking. So I'm changing these to be committing and rollbacking instead to be more obvious.

CMCDragonkai commented 1 year ago

In that case the dangling transaction attempts to release which will either commit or rollback. But this will cause an exception. This is unavoidable UNLESS we were to make is that committ or rollback is a noop if it is already committing or rollbacking.

So this change only applies to the case where committing or rollbacking has already started, and in which case those transactions should not result in any problems.

I'm not sure if that will fix the EFS, or we should apply a fix in EFS to ensure that the transaction is in fact committed or rollbacked before you stop the EFS (this is the better solution).

CMCDragonkai commented 1 year ago

Ok for now the EFS will still need to ensure that they are in fact committing or rollbacking any dangling transactions. But the db.stop() here is now more robust. @tegefaulkes