Merge pull request #1005 from jaten-molecula/bglimits

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

bluegreentx.go

@@ -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() {

dbshard.go

@@ -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
 }
 

txfactory.go

@@ -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)))
 }

txfactory_internal_test.go

@@ -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)
+		}
+	}
+}