fix(dgraph): Panic because of nil map in groups.go

[rahulgurnani]

Fixes DGRAPH-1935

delta.GroupChecksums is nil at this point in the code, so copying keys to this map causes the code to panic with message:
assignment to entry in nil map

---

This change is 

[martinmr]

Reviewed 1 of 1 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @manishrjain, @parasssh, and @vvbalaji-dgraph)

[martinmr]

actually, just remembered why this is done the way it is now. The existing delta might have checksums already. The fix should be to create the map if it is nil.

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @manishrjain, @parasssh, and @vvbalaji-dgraph)

[parasssh]

Agree with Martin. Lets make the map when the "delta" variable is declared on line 926.

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @manishrjain, @parasssh, and @vvbalaji-dgraph)

[parasssh]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, @rahulgurnani, and @vvbalaji-dgraph)

---

worker/groups.go, line 926 at r2 (raw file):

		for {
			var delta *pb.OracleDelta

can we do the make here itself? That way we dont have to do an if check in the for loop below.

[rahulgurnani]

@martinmr Addressed your review comment.

[parasssh]

I dont see it addressed.

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, @rahulgurnani, and @vvbalaji-dgraph)

[rahulgurnani]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, @parasssh, and @vvbalaji-dgraph)

---

worker/groups.go, line 926 at r2 (raw file):

I think if we keep it nearer to where it's used, code will be easier to understand. Also we may be allocating it would have to garbage collected if delta gets the GroupChecksums map from somewhere else.

[rahulgurnani]

I amended my previous commit, please check the git diff

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, @parasssh, and @vvbalaji-dgraph)

[parasssh]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, @rahulgurnani, and @vvbalaji-dgraph)

---

worker/groups.go, line 926 at r2 (raw file):

Previously, rahulgurnani (Rahul Gurnani) wrote…

I think if we keep it nearer to where it's used, code will be easier to understand. Also we may be allocating it would have to garbage collected if delta gets the GroupChecksums map from somewhere else.

I dont think that should be a concern. Currently we dont just assign it and make copy. I think make here is cleaner. Also, it is slightly better than doing the if check in the for loop on every iteration.

[rahulgurnani]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, @parasssh, and @vvbalaji-dgraph)

---

worker/groups.go, line 926 at r2 (raw file):

Previously, parasssh wrote…

I dont think that should be a concern. Currently we dont just assign it and make copy. I think make here is cleaner. Also, it is slightly better than doing the if check in the for loop on every iteration.

Okay, Updated the code

[parasssh]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, @parasssh, and @vvbalaji-dgraph)

[martinmr]

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @manishrjain, @parasssh, and @vvbalaji-dgraph)

[martinmr]

Can you run your tests locally. Lots of tests are failing so maybe the change is not as benign as we thought. But possible that this is just TeamCity acting up.

[CLAassistant]

All committers have signed the CLA.

[parasssh]

All CI tests pass locally.

[parasssh]

Can you run your tests locally. Lots of tests are failing so maybe the change is not as benign as we thought. But possible that this is just TeamCity acting up.

Actually, the code was faulty. Made the fix to do a make on the map just before using it in the for loop as Rahul had it earlier.