Tests: Do not leave behind state goroutines

[jarifibrahim]

A bunch of tests would start goroutines and never stop them. This
PR fixes that. There should not be any goroutines left after all the
tests are completed.

Fixes BADGER-221

---

This change is 

[ashish-goswami]

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @jarifibrahim and @manishrjain)

---

db.go, line 381 at r1 (raw file):

// Perform cleanup.

---

db.go, line 387 at r1 (raw file):

		db.bfCache.Close()
		db.orc.Stop()
		db.vlog.Close()

Not sure why we are doing clean up only for this error and not in case of other error like db.get(head). Also why are not closing compactions and memtable closer.

---

stream_writer.go, line 229 at r1 (raw file):

		return err
	}
	headWriter.closer.SignalAndWait()

Fix this as per comment given in other PR.

---

value_test.go, line 804 at r1 (raw file):

	require.NoError(t, err)
	require.NoError(t, db1.Close())

No need for this lin

[jarifibrahim]

Reviewable status: 2 of 6 files reviewed, 4 unresolved discussions (waiting on @ashish-goswami and @manishrjain)

---

db.go, line 381 at r1 (raw file):

Previously, ashish-goswami (Ashish Goswami) wrote…

// Perform cleanup.

Done.

---

db.go, line 387 at r1 (raw file):

Previously, ashish-goswami (Ashish Goswami) wrote…

Not sure why we are doing clean up only for this error and not in case of other error like db.get(head). Also why are not closing compactions and memtable closer.

This code is for db.Open(...) . If open failed, we should return nil. But we should also stop all the goroutines that were started for compaction and other operations. Blindly returning nil here would mean that the compactions keep running in the background and will keep crashing.

The code here stops all the operations that were started before reaching this point.

---

stream_writer.go, line 229 at r1 (raw file):

Previously, ashish-goswami (Ashish Goswami) wrote…

Fix this as per comment given in other PR.

Done.

---

value_test.go, line 804 at r1 (raw file):

Previously, ashish-goswami (Ashish Goswami) wrote…

No need for this lin

Done.

[ashish-goswami]

One comment

Reviewable status: 1 of 5 files reviewed, 1 unresolved discussion (waiting on @ashish-goswami, @jarifibrahim, and @manishrjain)

---

db.go, line 387 at r1 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

This code is for db.Open(...) . If open failed, we should return nil. But we should also stop all the goroutines that were started for compaction and other operations. Blindly returning nil here would mean that the compactions keep running in the background and will keep crashing.

The code here stops all the operations that were started before reaching this point.

Exactly. But I think we are not closing compactions and memtable closer. In db.close() it is done as

db.stopMemoryFlush()
db.stopCompactions()

Also why we are not doing clean up in case db.get(headKey) returns error.

[jarifibrahim]

Dismissed @ashish-goswami from a discussion.
Reviewable status: 1 of 5 files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @manishrjain)

---

db.go, line 387 at r1 (raw file):

Previously, ashish-goswami (Ashish Goswami) wrote…

Exactly. But I think we are not closing compactions and memtable closer. In db.close() it is done as

db.stopMemoryFlush()
db.stopCompactions()

Also why we are not doing clean up in case db.get(headKey) returns error.

I see what you mean. I've fixed that.

[ashish-goswami]

Reviewed 5 of 6 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)