fix(restore): Bump uid and namespace after restore

[ahsanbarkati]

We were not bumping the lease after restore. This PR fixes it.

Note: The introduced functionality of being able to bump to a specific value can also be used in live loader. I am not making that change in this PR to keep this PR simple.

---

This change is 

[manishrjain]

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ahsanbarkati)

---

worker/online_restore.go, line 410 at r1 (raw file):

	}

	if err := leaseID(mr.maxUid, pb.Num_UID); err != nil {

It would make sense to say bumpTo. So, even if there are multiple calls, so it would deal with race conditions better.

[NamanJain8]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ahsanbarkati)

---

worker/restore_map.go, line 324 at r1 (raw file):

		if ns > maxNs {
			maxNs = ns
		}

Use x.Max()

[NamanJain8]

Reviewable status: 1 of 7 files reviewed, 6 unresolved discussions (waiting on @ahsanbarkati and @manishrjain)

---

dgraph/cmd/zero/assign.go, line 247 at r3 (raw file):

	}

	if num.GetBump() {

Shouldn't this calculation be done on the leader?

---

dgraph/cmd/zero/assign.go, line 256 at r3 (raw file):

		num.Val = 0
		if cur < req {
			num.Val = req - cur

What if cur >= req, shouldn't we return from here itself and not do a lease?

---

dgraph/cmd/zero/zero_test.go, line 73 at r3 (raw file):

	res, err = zc.AssignIds(ctx, &pb.Num{Val: 100000, Type: pb.Num_UID, Bump: true})
	if err != nil {
		require.Error(t, err, "Nothing to be leased")

Maybe bump it to a value greater than the one leased in the earlier AssignIds. So that we know if we should expect an error or not.

---

dgraph/cmd/zero/zero_test.go, line 84 at r3 (raw file):

	// If bump request is less than maxLease, then it should result in no-op.
	res, err = zc.AssignIds(ctx, &pb.Num{Val: 10, Type: pb.Num_UID, Bump: true})
	require.Error(t, err, "Nothing to be leased")

require.Contains

---

worker/online_restore.go, line 349 at r2 (raw file):

		return errors.Wrapf(err, "Failed to map the backup files")
	}
	glog.Infof("Backup map phase is complete. Map result is: %+v\n", mapRes)

nit: No need for \n

[ahsanbarkati]

Reviewable status: 1 of 7 files reviewed, 5 unresolved discussions (waiting on @manishrjain and @NamanJain8)

---

dgraph/cmd/zero/assign.go, line 247 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

Shouldn't this calculation be done on the leader?

Fixed

---

dgraph/cmd/zero/assign.go, line 256 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

What if cur >= req, shouldn't we return from here itself and not do a lease?

In that case, the lease request would have been with val = 0 so it would have returned anyway. Making it return here itself for clarity.

---

dgraph/cmd/zero/zero_test.go, line 73 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

Maybe bump it to a value greater than the one leased in the earlier AssignIds. So that we know if we should expect an error or not.

Done.

---

dgraph/cmd/zero/zero_test.go, line 84 at r3 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

require.Contains

Done.

---

worker/online_restore.go, line 410 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

It would make sense to say bumpTo. So, even if there are multiple calls, so it would deal with race conditions better.

Done.