Access groupi.gid atomically

[animesh2049]

We were accessing groupi.gid without synchronization. This commit ensures that groupi.gid is always accessed atomically. Fixes #3343

This change is 

[CLAassistant]

All committers have signed the CLA.

[martinmr]

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

---

dgraph/cmd/alpha/run.go, line 559 at r1 (raw file):

	// TODO (animesh): Not sure why this was done in the first place
	// _ = numShutDownSig

This looks like it's assigning the value of numShutDownSig to an anonymous variable. It shouldn't have any effect. I think it's safe to remove in this PR.

---

worker/groups.go, line 293 at r1 (raw file):

func (g *groupi) ServesGroup(gid uint32) bool {
	// No need to acquire the lock on g because gid is always
	// accessed atomically via groupId() function

Not sure if this comment is really needed. I suppose it would be useful to someone who remembered the previous version. I'll let you decide if you want to keep it.

In case you want to keep it two comments:

• Period at the end.
• Also, if you can fit the whole comment in a single line (100 columns) it'd be better.

[animesh2049]

---

worker/groups.go, line 293 at r1 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

Not sure if this comment is really needed. I suppose it would be useful to someone who remembered the previous version. I'll let you decide if you want to keep it.

In case you want to keep it two comments:

• Period at the end.
• Also, if you can fit the whole comment in a single line (100 columns) it'd be better.

I have removed the comment.

[mangalaman93]

Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @manishrjain and @martinmr)

---

dgraph/cmd/alpha/run.go, line 559 at r1 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

	// TODO (animesh): Not sure why this was done in the first place
	// _ = numShutDownSig

This looks like it's assigning the value of numShutDownSig to an anonymous variable. It shouldn't have any effect. I think it's safe to remove in this PR.

Done

---

worker/groups.go, line 293 at r1 (raw file):

Previously, animesh2049 (Animesh Chandra Pathak) wrote…

I have removed the comment.

Done

[mangalaman93]

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

[manishrjain]

@animesh2049 FYI, this PR needed more approvals before merging. Something to keep in mind for next time.

[mangalaman93]

Martin had approved it, so we went through with it. Will be careful in future.

[animesh2049]

sure @manishrjain
I will keep this in mind.