delete item immediately

[poonai]

Delete was happening concurrently in a separate goroutine. This will make the delete happen immediately But the pitfall is that if there is a set in the channel it'll bring the item again. So, we're pushing to the channel as well to maintain the order.
Signed-off-by: balaji balaji@dgraph.io

---

This change is 

[jarifibrahim]

This PR needs a description.

[poonai]

@jarifibrahim updated

[manishrjain]

No. This shouldn't happen. In fact, please do add this following explanation as a comment.

If we have pushed a Set to a channel, it would be applied slightly later. In current code, if a deletion immediately follows that Set, it would be applied in the same order. With this PR, the deletion would be applied before the Set, which would be wrong.

Please do add a comment of that nature. CC: @karlmcguire to verify my understanding.

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @jarifibrahim, @karlmcguire, and @manishrjain)

[manishrjain]

Do address the comment.

Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @balajijinnah, @jarifibrahim, and @karlmcguire)

---

cache.go, line 220 at r2 (raw file):

	keyHash, conflictHash := c.keyToHash(key)
	c.store.Del(keyHash, conflictHash)
	c.setBuf <- &item{

Please add a comment here as explained in the last review.

[poonai]

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @jarifibrahim, @karlmcguire, and @manishrjain)

---

cache.go, line 220 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Please add a comment here as explained in the last review.

Done.

[jarifibrahim]

Please undo the change in the test or add a new one instead :)

Reviewed 2 of 3 files at r1, 1 of 1 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @balajijinnah, @karlmcguire, and @manishrjain)

---

cache.go, line 220 at r2 (raw file):

Previously, balajijinnah (balaji) wrote…

Done.

Nit: Grammatical error, (use Grammarly, it helps)

If we've set an item, it would be applied slightly later. So it is important that we push the same item to `setBuf` with the deletion flag. This ensures that if a set is followed by a delete, it will be applied in the correct order.

---

cache_test.go, line 174 at r3 (raw file):

	}
	key, conflict = z.KeyToHash(1)
	c.setBuf <- &item{

Why was this removed? This seems to be testing item deletion.

If this has to be removed, please add a new test for delete. The existing test for delete is not as comprehensive as this one.

[poonai]

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @jarifibrahim, @karlmcguire, and @manishrjain)

---

cache_test.go, line 174 at r3 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

Why was this removed? This seems to be testing item deletion.

If this has to be removed, please add a new test for delete. The existing test for delete is not as comprehensive as this one.

Done.

[jarifibrahim]

Reviewed 2 of 2 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @karlmcguire and @manishrjain)