refactor: removeTx responsible for getting/releasing its own lock

We change nothing substantive here, except that there's a window
between when a write transaction updates the root pages and when
it removes itself from the db tx list and possibly causes a checkpoint
where it's not holding the db lock.

The issue here is that we want to be able to *keep* the lock but
still return, so no one else can start transactions, but the specific
Rollback or Commit that removed the last outstanding transaction
doesn't block forever. This will, later, allow us to exercise
finer-grained control over when we allow transactions. This is
a separate commit so we can run the test suite against it, and
verify that this part in particular didn't break anything.
This commit is contained in:
Seebs 2021-11-19 13:14:42 -06:00 committed by Matthew Jaffee
parent 5c889c72bd
commit a631e25dc5
2 changed files with 15 additions and 6 deletions

View file

@ -503,8 +503,13 @@ func (db *DB) Begin(writable bool) (_ *Tx, err error) {
return tx, nil
}
// removeTx removes an active transaction from the database.
// removeTx removes an active transaction from the database. it obtains
// the db lock, and currently drops it, but will later possibly be leaving
// it retained by an asynchronous op that wants to happen before we start
// running new tx.
func (db *DB) removeTx(tx *Tx) error {
db.mu.Lock()
defer db.mu.Unlock()
// Release writer lock if tx is writable.
if tx.writable {
tx.db.rwmu.Unlock()

View file

@ -109,20 +109,24 @@ func (tx *Tx) Commit() error {
// future plan: after checkpoint is moved to background
// or not every removeTx, then we can move the
// tx.db.rootRecords = tx.rootRecords into removeTx().
//
// ... or maybe not: let's do that part here, and then removeTx
// may or may not start a checkpoint, possibly asynchronously.
//
// avoid race detector firing on a write race here
// vs the read of rootRecords at db.Begin()
// vs the read of rootRecords at db.Begin(), then release
// the lock, because we need removeTx to grab the lock to
// work, but if it wants to checkpoint, it wants to be able to return
// to us here and still be holding the lock.
tx.db.mu.Lock()
defer tx.db.mu.Unlock()
tx.db.rootRecords = tx.rootRecords
tx.db.pageMap = tx.pageMap
tx.db.walPageN = tx.walPageN
tx.db.mu.Unlock()
return tx.db.removeTx(tx)
}
// Disconnect transaction from DB.
tx.db.mu.Lock()
defer tx.db.mu.Unlock()
return tx.db.removeTx(tx)
}