StreamWriter: Close head writer #1347
Conversation
The head writer was not being closed which would leak the `handleRequests` go routine. This PR fixes it by closing the head writer.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @manishrjain)
There was a problem hiding this comment.
One comment. Also looks like one of the test case is getting into deadlock, please check that as well.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jarifibrahim and @manishrjain)
stream_writer.go, line 229 at r1 (raw file):
return err } headWriter.closer.SignalAndWait()
SignalAndWait should be called before Done(), we are doing same in case of other writers as well.
There was a problem hiding this comment.
There's no deadlock. Badger has too many tests and they do not complete in 15 minutes if you run them with the -race flag.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @manishrjain)
stream_writer.go, line 229 at r1 (raw file):
Previously, ashish-goswami (Ashish Goswami) wrote…
SignalAndWaitshould be called beforeDone(), we are doing same in case of other writers as well.
No no. if I call signalAndWait before calling Done we would end up in a deadlock. These are serially executed and done would never be called.
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @manishrjain)
stream_writer.go, line 229 at r1 (raw file):
Previously, jarifibrahim (Ibrahim Jarif) wrote…
No no. if I call
signalAndWaitbefore callingDonewe would end up in a deadlock. These are serially executed anddonewould never be called.
Sorry, I got confused between writer.Done and closer.Done. You are correct, I've made the change.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:
This PR adds the following changes from **Master branch to the dgraph-maintenance branch**. ``` 25fd0ef Update ristretto to commit f66de99 (#1391) fac972f Update head while replaying value log (#1372) e5fd05a Rework DB.DropPrefix (#1381) adeb842 Fix assert in background compression and encryption. (#1366) ``` The **master and dgraph-maintenace branch** have following difference `+` (in green) means commit missing on dgraph-maintenance `-` (in red) means the commit exists. ```diff + 079f5ae DefaultOptions: Set KeepL0InMemory to false (#1345) + 7e19cac Add immudb to the project list (#1341) + da80eb9 Iterator: Always add key to txn.reads (#1328) + a7e239e StreamWriter: Close head writer (#1347) + 543f353 Fix build on golang tip (#1355) + fd89894 Compaction: Expired keys and delete markers are never purged (#1354) + 056d859 Support disabling conflict detection (#1344) + b762832 Tests: Do not leave behind state goroutines (#1349) + b2267c2 Restore: Account for value size as well (#1358) + 14386ac GC: Consider size of value while rewriting (#1357) + c45d966 Fix assert in background compression and encryption. (#1366) + dd332b0 Avoid panic in filltables() (#1365) + 3f4761d Force KeepL0InMemory to be true when InMemory is true (#1375) + d37ce36 Tests: Use t.Parallel in TestIteratePrefix tests (#1377) + 158d927 Remove second initialization of writech in Open (#1382) + 675efcd Increase default valueThreshold from 32B to 1KB (#1346) + 3042e37 pre allocate cache key for the block cache and the bloom filter cache (#1371) + e013bfd Rework DB.DropPrefix (#1381) + 509de73 Update head while replaying value log (#1372) + 09dfa66 Update ristretto to commit f66de99 (#1391) + 717b89c Enable cross-compiled 32bit tests on TravisCI (#1392) ``` <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/dgraph-io/badger/1398) <!-- Reviewable:end -->
The head writer was not being closed which would leak the handleRequests go routine. This PR fixes it by closing the head writer.
The head writer was not being closed which would leak the
handleRequestsgo routine. This PR fixes it by closing the headwriter.
This change is