leveldb: when closing a DB, wait for any in-flight compaction to

complete before cleaning up the table cache.

Fixes issue 14.

LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://codereview.appspot.com/99800043

leveldb/compaction.go

@@ -142,10 +142,9 @@ func (c *compaction) isBaseLevelForUkey(userCmp db.Comparer, ukey []byte) bool {
 //
 // d.mu must be held when calling this.
 func (d *DB) maybeScheduleCompaction() {
-	if d.compacting {
+	if d.compacting || d.closed {
 		return
 	}
-	// TODO: check if db is shutting down.
 	// TODO: check for manual compactions.
 	if d.imm == nil {
 		v := d.versions.currentVersion()

leveldb/leveldb.go

@@ -76,6 +76,8 @@ type DB struct {
 	compactionCond sync.Cond
 	compacting     bool
 
+	closed bool
+
 	pendingOutputs map[uint64]struct{}
 }
 
@@ -180,16 +182,19 @@ func (d *DB) Find(key []byte, opts *db.ReadOptions) db.Iterator {
 }
 
 func (d *DB) Close() error {
-	err := d.tableCache.Close()
 	d.mu.Lock()
 	defer d.mu.Unlock()
-	if d.fileLock == nil {
-		return err
+	if d.closed {
+		return nil
 	}
+	for d.compacting {
+		d.compactionCond.Wait()
+	}
+	err := d.tableCache.Close()
 	err = firstError(err, d.log.Close())
 	err = firstError(err, d.logFile.Close())
 	err = firstError(err, d.fileLock.Close())
-	d.fileLock = nil
+	d.closed = true
 	return err
 }
 

leveldb/leveldb_test.go

@@ -427,58 +427,71 @@ func TestOpenCloseOpenClose(t *testing.T) {
 		FileSystem: memfs.New(),
 	}
 
-	for _, length := range []int{-1, 0, 1, 1000, 10000, 100000} {
-		dirname := strconv.Itoa(length)
-		got, xxx := []byte(nil), ""
-		if length >= 0 {
-			xxx = strings.Repeat("x", length)
-		}
+	for _, startFromEmpty := range []bool{false, true} {
+		for _, length := range []int{-1, 0, 1, 1000, 10000, 100000} {
+			dirname := "sharedDatabase"
+			if startFromEmpty {
+				dirname = "startFromEmpty" + strconv.Itoa(length)
+			}
 
-		d0, err := Open(dirname, opts)
-		if err != nil {
-			t.Errorf("length=%d: Open #0: %v", length, err)
-			continue
-		}
-		if length >= 0 {
-			err = d0.Set([]byte("key"), []byte(xxx), nil)
+			got, xxx := []byte(nil), ""
+			if length >= 0 {
+				xxx = strings.Repeat("x", length)
+			}
+
+			d0, err := Open(dirname, opts)
 			if err != nil {
-				t.Errorf("length=%d: Set: %v", length, err)
+				t.Errorf("sfe=%t, length=%d: Open #0: %v",
+					startFromEmpty, length, err)
 				continue
 			}
-		}
-		err = d0.Close()
-		if err != nil {
-			t.Errorf("length=%d: Close #0: %v", length, err)
-			continue
-		}
-
-		// TODO: make the second Open recover (without a fatal "corrupt log
-		// file" error) even if the d0 database was not closed but the xxx
-		// value is large enough to write a partial record. Writing to the
-		// database should not corrupt it even if the writer process was
-		// killed part-way through.
-
-		d1, err := Open(dirname, opts)
-		if err != nil {
-			t.Errorf("length=%d: Open #1: %v", length, err)
-			continue
-		}
-		if length >= 0 {
-			got, err = d1.Get([]byte("key"), nil)
+			if length >= 0 {
+				err = d0.Set([]byte("key"), []byte(xxx), nil)
+				if err != nil {
+					t.Errorf("sfe=%t, length=%d: Set: %v",
+						startFromEmpty, length, err)
+					continue
+				}
+			}
+			err = d0.Close()
 			if err != nil {
-				t.Errorf("length=%d: Get: %v", length, err)
+				t.Errorf("sfe=%t, length=%d: Close #0: %v",
+					startFromEmpty, length, err)
 				continue
 			}
-		}
-		err = d1.Close()
-		if err != nil {
-			t.Errorf("length=%d: Close #1: %v", length, err)
-			continue
-		}
 
-		if length >= 0 && string(got) != xxx {
-			t.Errorf("length=%d: got value differs from set value", length)
-			continue
+			// TODO: make the second Open recover (without a fatal "corrupt log
+			// file" error) even if the d0 database was not closed but the xxx
+			// value is large enough to write a partial record. Writing to the
+			// database should not corrupt it even if the writer process was
+			// killed part-way through.
+
+			d1, err := Open(dirname, opts)
+			if err != nil {
+				t.Errorf("sfe=%t, length=%d: Open #1: %v",
+					startFromEmpty, length, err)
+				continue
+			}
+			if length >= 0 {
+				got, err = d1.Get([]byte("key"), nil)
+				if err != nil {
+					t.Errorf("sfe=%t, length=%d: Get: %v",
+						startFromEmpty, length, err)
+					continue
+				}
+			}
+			err = d1.Close()
+			if err != nil {
+				t.Errorf("sfe=%t, length=%d: Close #1: %v",
+					startFromEmpty, length, err)
+				continue
+			}
+
+			if length >= 0 && string(got) != xxx {
+				t.Errorf("sfe=%t, length=%d: got value differs from set value",
+					startFromEmpty, length)
+				continue
+			}
 		}
 	}
 }