Fix request increment ref bug

[jarifibrahim]

The sendToWriteCh() method would increment the ref of a request by 2
(one for the write and another for the publisher). The ref value was
always decremented by the doWrite() function but the publisher would
decrement it only if there were any subscribers.

This would mean that we ALWAYS create new request object in the requestPool but never put them back into the pool.
https://github.com/dgraph-io/badger/blob/eef7c122607e5c2cc60e14eb20c883efa3e6855b/value.go#L1204-L1211

With this commit, we increment the request ref for the publisher only if
there are any actual subscribers.

The adding print statements around the requestPool.Get() and requestPool.Put() functions shows the request object were never being added to the pool.

---

This change is 

[coveralls]

Coverage increased (+0.4%) to 77.908% when pulling 098d2bb on ibrahim/requestPool-fix into ffb3450 on master.

[coveralls]

Coverage increased (+0.07%) to 77.45% when pulling 9af7e00 on ibrahim/requestPool-fix into eef7c12 on master.

[martinmr]

Looks good to me but get final approval from Manish.

Reviewed 3 of 3 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ashish-goswami and @manishrjain)

[ashish-goswami]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jarifibrahim and @manishrjain)

---

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

	db.elog.Printf("Sending updates to subscribers")
	db.pub.sendUpdates(reqs)

Can we add a comment saying that we increase reference of the requests inside sendUpdates? Since we are increasing/decreasing reference twice, we have to make sure reference is 0 only at end. Hence order of statements matter here.

[poonai]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jarifibrahim and @manishrjain)

[manishrjain]

The increment should happen upfront. This PR would create a race cond.

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

---

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

	req.Wg.Add(1)
	req.IncrRef()     // for db write
	db.writeCh <- req // Handled in doWrites.

What if db write happens before publisher has a chance to pick it up. You should still increment it here, but then once publisher gets it, it can decr ref it.

---

publisher.go, line 155 at r1 (raw file):

func (p *publisher) sendUpdates(reqs requests) {
	if p.noOfSubscribers() != 0 {
		reqs.IncrRef()

This is too late. This would create a race cond between db.write.

[jarifibrahim]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @jarifibrahim and @manishrjain)

---

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

Previously, manishrjain (Manish R Jain) wrote…

What if db write happens before publisher has a chance to pick it up. You should still increment it here, but then once publisher gets it, it can decr ref it.

That will not happen because we sendupdates to subscribers before we mark the request as done. See this line
https://github.com/dgraph-io/badger/blob/5f94f68074b4a78483afd63c3f8a67141c9aeab8/db.go#L672-L681

I can make changes to increment it in sendToWriteCh and decrement it in sendUpdates

[manishrjain]

Got the explanation.

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @jarifibrahim and @manishrjain)