From 7dc58671422437d07cd7a67c5d36ad2fde2ffb25 Mon Sep 17 00:00:00 2001 From: cmasone-attic Date: Fri, 4 Nov 2016 16:25:53 -0700 Subject: [PATCH] Close and destroy databases being left open during tests (#2806) There were several tests in the Database suites that were failing to close test Databases that had orderedChunkCaches in them (backed by levelDBs). Close them. I was ALSO failing to destroy the cache used in LocalDatabase instances only while testing Pull(). That's cleared up now as well. --- go/datas/database_test.go | 8 ++++++-- go/datas/local_batch_store.go | 5 +++++ go/datas/local_database.go | 8 ++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/go/datas/database_test.go b/go/datas/database_test.go index ed70628d89..8ec78b41eb 100644 --- a/go/datas/database_test.go +++ b/go/datas/database_test.go @@ -111,12 +111,14 @@ func (suite *DatabaseSuite) TestCommitProperlyTracksRoot() { id1, id2 := "testdataset", "othertestdataset" db1 := suite.makeDb(suite.cs) + defer db1.Close() ds1 := db1.GetDataset(id1) ds1HeadVal := types.String("Commit value for " + id1) ds1, err := db1.CommitValue(ds1, ds1HeadVal) suite.NoError(err) db2 := suite.makeDb(suite.cs) + defer db2.Close() ds2 := db2.GetDataset(id2) db2HeadVal := types.String("Commit value for " + id2) ds2, err = db2.CommitValue(ds2, db2HeadVal) @@ -187,9 +189,9 @@ func (suite *DatabaseSuite) TestDatabaseCommit() { // Get a fresh database, and verify that both datasets are present newDB := suite.makeDb(suite.cs) + defer newDB.Close() datasets2 := newDB.Datasets() suite.Equal(uint64(2), datasets2.Len()) - newDB.Close() } func newOpts(parents ...types.Value) CommitOptions { @@ -279,11 +281,11 @@ func (suite *DatabaseSuite) TestDatabaseDelete() { // Get a fresh database, and verify that only ds1 is present newDB := suite.makeDb(suite.cs) + defer newDB.Close() datasets = newDB.Datasets() suite.Equal(uint64(1), datasets.Len()) _, present = newDB.GetDataset(datasetID2).MaybeHeadRef() suite.True(present, "Dataset %s should be present", datasetID2) - newDB.Close() } type waitDuringUpdateRootChunkStore struct { @@ -314,6 +316,7 @@ func (suite *DatabaseSuite) TestCommitWithConcurrentChunkStoreUse() { // Craft DB that will allow me to move the backing ChunkStore while suite.db isn't looking w := &waitDuringUpdateRootChunkStore{suite.cs, nil} db := suite.makeDb(w) + defer db.Close() // Concurrent change, but to some other dataset. This shouldn't stop changes to ds1. // ds1: |a| <- |b| @@ -368,6 +371,7 @@ func (suite *DatabaseSuite) TestDeleteWithConcurrentChunkStoreUse() { // Craft DB that will allow me to move the backing ChunkStore while suite.db isn't looking w := &waitDuringUpdateRootChunkStore{suite.cs, nil} db := suite.makeDb(w) + defer db.Close() // Concurrent change, to move root out from under my feet: // ds1: |a| <- |b| <- |e| diff --git a/go/datas/local_batch_store.go b/go/datas/local_batch_store.go index 8c173cbfe5..399ba30d28 100644 --- a/go/datas/local_batch_store.go +++ b/go/datas/local_batch_store.go @@ -127,6 +127,11 @@ func (lbs *localBatchStore) FlushAndDestroyWithoutClose() { lbs.unwrittenPuts.Destroy() } +// Destroy blows away lbs' cache of unwritten chunks without flushing. Used when the owning Database is closing and it isn't semantically correct to flush. +func (lbs *localBatchStore) Destroy() { + lbs.unwrittenPuts.Destroy() +} + // Close is supposed to close the underlying ChunkStore, but the only place localBatchStore is currently used wants to keep the underlying ChunkStore open after it's done with lbs. Hence, the above method and the panic() here. func (lbs *localBatchStore) Close() error { panic("Unreached") diff --git a/go/datas/local_database.go b/go/datas/local_database.go index 536fecf2c8..723da3f54c 100644 --- a/go/datas/local_database.go +++ b/go/datas/local_database.go @@ -67,3 +67,11 @@ func (ldb *LocalDatabase) validatingBatchStore() types.BatchStore { } return ldb.vbs } + +func (ldb *LocalDatabase) Close() error { + if ldb.vbs != nil { + ldb.vbs.Destroy() + ldb.vbs = nil + } + return ldb.databaseCommon.Close() +}