Fix blue-green Tx cleanup and document single import at once

- correct string constants for txtype so that
  blue-green cleanup correctly detects when
  2nd transaction in a pair has Committed and
  thus the blue-green RWMutex can be relased

- test that txtype.String() is consistent with
  the corresponding string constants.

- document in bluegreentx.go the current limitations
  of blue-green testing: only one github archive import
  (a single writing client) is supported by blue-green
  testing. Multiple importers will deadlock eventually
  on the DBShard.mut RWMutex. We could fix this by
  ordering the write locks and obtaining them in
  strictly increasing order (by shard number), but
  that would require alot of change to the executor
  and that would introduce more risk for a test-only
  pathway.
This commit is contained in:
Jason E. Aten 2020-10-19 18:28:45 -05:00
parent 2ebff707a9
commit d9783406bd
4 changed files with 37 additions and 6 deletions

View file

@ -36,6 +36,15 @@ import (
// Do not run with go test -race and expect it to be race free with RoaringTx
// on one arm.
//
// Note: using the dbshard.go DBShard.mut RWMutex to begin and end
// both the A and B transactions atomically, we support a single importer and
// lots of readers running under blue-green transactions. Two writers a.k.a. two
// github ingests at once will deadlock eventually, but I think that may be asking
// for more than we want to test under blue-green, as it would require a bunch of
// test-only internal executor logic that could mess with the production path.
// So, for now, a limitation on blue green tests is that they be single
// writer/single importer going at once.
//
type blueGreenTx struct {
a Tx
b Tx // b's output is returned
@ -111,6 +120,9 @@ func (b *blueGreenRegistry) finishedTx(tx *blueGreenTx) {
sn := tx.Sn()
delete(b.m, sn)
//vv("blueGreenRegistry deleted _sn_ %v", sn)
// Note that a tx.o.dbs.Cleanup(tx) call should not be needed,
// because the individual tx will call cleanup themselves.
}
func (b *blueGreenRegistry) Close() {

View file

@ -68,10 +68,8 @@ type DBShard struct {
Shard uint64
Open bool
// With RWMutex, the
// writer who calls Lock() automatically gets priority over
// any reader who arrives later, even if the lock is held
// by a reader to start with.
// With RWMutex, the blue-green Tx can start and commit
// atomically.
mut sync.RWMutex
types []txtype
@ -136,14 +134,17 @@ func (dbs *DBShard) Cleanup(tx Tx) {
if dbs == nil {
return // some tests are using Tx only, no dbs available.
}
//vv("top of DBShard %v Cleanup for tx.Sn = %v; dbs=%p; is 2nd: %v; type='%v'; dbs.stypes='%#v'", dbs.Shard, tx.Sn(), dbs, tx.Type() == dbs.stypes[1], tx.Type(), dbs.stypes)
if !dbs.hasRoaring {
if dbs.isBlueGreen {
// only release on the 2nd Tx's cleanup
if tx.Type() == dbs.stypes[1] {
if tx.Readonly() {
dbs.mut.RUnlock()
//vv("gid %v released read-lock on shard %v", curGID(), dbs.Shard)
} else {
dbs.mut.Unlock()
//vv("gid %v released write-lock on shard %v", curGID(), dbs.Shard)
}
}
}
@ -158,7 +159,9 @@ func (dbs *DBShard) NewTx(write bool, initialIndexName string, o Txo) (tx Tx, er
if !dbs.hasRoaring {
if write {
dbs.mut.Lock()
//vv("shard %v was write locked by gid %v; stack =\n%v", dbs.Shard, curGID(), stack())
} else {
//vv("shard %v about to be read locked by gid %v; stack=\n%v", dbs.Shard, curGID(), stack())
dbs.mut.RLock()
}
}
@ -183,6 +186,7 @@ func (dbs *DBShard) NewTx(write bool, initialIndexName string, o Txo) (tx Tx, er
}
// blue green
tx, err = dbs.per.txf.newBlueGreenTx(txns[0], txns[1], o.Index, o), nil
//vv("dbshard returning blue-green tx sn %v", tx.Sn())
return
}

View file

@ -846,9 +846,9 @@ func (ty txtype) String() string {
case rbfTxn:
return "rbf"
case lmdbTxn:
return "lmdbTxn"
return "lmdb"
case boltTxn:
return "boltTxn"
return "bolt"
}
panic(fmt.Sprintf("unhandled ty '%v' in txtype.String()", int(ty)))
}

View file

@ -380,3 +380,18 @@ func Test_TxFactory_verifyBlueEqualsGreen(t *testing.T) {
}
}
}
func Test_TxFactory_verifyStringConstantsMatch(t *testing.T) {
// txtype.String() method MUST return strings that match
// our const definitions at the top of txfactory.go, or
// else blue-green transactions cannot determine when
// the second transaction is being released in dbshard.go.
check := []txtype{roaringTxn, rbfTxn, lmdbTxn, boltTxn}
expect := []string{RoaringTxn, RBFTxn, LmdbTxn, BoltTxn}
for i, chk := range check {
obs := chk.String()
if obs != expect[i] {
t.Fatalf("expected '%v' but got '%v'", expect[i], obs)
}
}
}