Fix bug in backup KeyToList function.

[martinmr]

Previously, the KeyToList function used during backup assumed that
ReadPostingList takes you to the next key so calling iterator.Next
manually is not needed. This is not the case during backups.
The fix is either to make ReadPostingList call iterator.Next after reading a
complete posting list.

The bug was being triggered by the bulk loader because inserting data via
mutations creates delta posting lists but the bulk loader creates complete
posting lists.

Fixes #3831

---

This change is 

[martinmr]

Backups succeed now

alpha1    | I0816 23:59:26.781008       1 backup_ee.go:33] Backup request: group 1 at 200001
alpha1    | I0816 23:59:26.781799       1 file_handler.go:61] Using file path: "/data/dgraph.20190816.235926.777/r200001-g1.backup"
alpha1    | I0816 23:59:26.794940       1 log.go:34] Dgraph.Backup Created batch of size: 6.1 kB in 534.03µs.
alpha1    | I0816 23:59:26.818160       1 log.go:34] Dgraph.Backup Created batch of size: 61 kB in 10.225401ms.
alpha1    | I0816 23:59:26.818369       1 log.go:34] Dgraph.Backup Created batch of size: 17 kB in 177.008µs.
alpha1    | I0816 23:59:26.834890       1 log.go:34] Dgraph.Backup Created batch of size: 65 kB in 11.009294ms.
alpha1    | I0816 23:59:26.840194       1 log.go:34] Dgraph.Backup Created batch of size: 61 kB in 4.617674ms.
alpha1    | I0816 23:59:26.951995       1 log.go:34] Dgraph.Backup Created batch of size: 659 kB in 17.992496ms.
alpha1    | I0816 23:59:27.021885       1 log.go:34] Dgraph.Backup Created batch of size: 1.2 MB in 69.088409ms.
alpha1    | I0816 23:59:27.147279       1 log.go:34] Dgraph.Backup Created batch of size: 1.7 MB in 124.33339ms.
alpha1    | I0816 23:59:27.577322       1 log.go:34] Dgraph.Backup Created batch of size: 4.3 MB in 188.656489ms.
alpha1    | I0816 23:59:28.284864       1 log.go:34] Dgraph.Backup Created batch of size: 21 MB in 683.079873ms.
alpha1    | I0816 23:59:28.284886       1 log.go:34] Dgraph.Backup Time elapsed: 01s, bytes sent: 29 MB, speed: 29 MB/sec
alpha1    | I0816 23:59:28.901229       1 log.go:34] Dgraph.Backup Created batch of size: 23 MB in 587.365111ms.
alpha1    | I0816 23:59:29.344959       1 log.go:34] Dgraph.Backup Created batch of size: 12 MB in 435.545242ms.
alpha1    | I0816 23:59:29.535516       1 log.go:34] Dgraph.Backup Created batch of size: 4.3 MB in 187.773509ms.
alpha1    | I0816 23:59:29.710275       1 log.go:34] Dgraph.Backup Created batch of size: 4.2 MB in 173.453127ms.
alpha1    | I0816 23:59:29.875095       1 log.go:34] Dgraph.Backup Created batch of size: 4.2 MB in 164.478751ms.
alpha1    | I0816 23:59:30.041587       1 log.go:34] Dgraph.Backup Created batch of size: 4.2 MB in 166.193187ms.
alpha1    | I0816 23:59:30.209295       1 log.go:34] Dgraph.Backup Created batch of size: 4.2 MB in 167.488156ms.
alpha1    | I0816 23:59:30.209317       1 log.go:34] Dgraph.Backup Time elapsed: 03s, bytes sent: 85 MB, speed: 28 MB/sec
alpha1    | I0816 23:59:30.424632       1 log.go:34] Dgraph.Backup Created batch of size: 4.3 MB in 177.80734ms.
alpha1    | I0816 23:59:30.671908       1 log.go:34] Dgraph.Backup Created batch of size: 4.3 MB in 161.849646ms.
alpha1    | I0816 23:59:30.910795       1 log.go:34] Dgraph.Backup Created batch of size: 4.3 MB in 158.741676ms.
alpha1    | I0816 23:59:30.910814       1 log.go:34] Dgraph.Backup Time elapsed: 04s, bytes sent: 98 MB, speed: 24 MB/sec
alpha1    | I0816 23:59:30.928619       1 log.go:34] Dgraph.Backup Created batch of size: 469 kB in 17.59542ms.
alpha1    | I0816 23:59:30.928631       1 log.go:34] Dgraph.Backup Sent 1179357 keys
alpha1    | I0816 23:59:30.928641       1 backup.go:149] Backup group 1 version: 200001
alpha1    | I0816 23:59:30.963137       1 backup.go:158] Backup complete: group 1 at 200001
alpha1    | I0816 23:59:30.963252       1 file_handler.go:61] Using file path: "/data/dgraph.20190816.235926.777/manifest.json"
alpha1    | I0816 23:59:30.966281       1 backup.go:189] Backup completed OK.

[gitlw]

@martinmr Can you please fix the failed tests?

[gitlw]

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

---

ee/backup/backup.go, line 272 at r1 (raw file):

		}

		itr.Next()

Can this be moved up into the for loop line?

[martinmr]

Fixed the tests. Turns out that it.Next should be called inside of ReadPostingList. Otherwise, it will be called twice when reading a delta posting list and some data will be excluded from the backup.

I'll update the description accordingly.

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @gitlw and @manishrjain)

---

ee/backup/backup.go, line 272 at r1 (raw file):

Previously, gitlw (Lucas Wang) wrote…

Can this be moved up into the for loop line?

Not relevant anymore.

addressed review

[martinmr]

Running the tests again. Hopefully this change does not break anything since ReadPostingList has been changed.

[manishrjain]

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

---

posting/mvcc.go, line 170 at r3 (raw file):

			}
			l.minTs = item.Version()
			it.Next()

This Next shouldn't be here. Otherwise, we'll have to do Next for every return call here (BitEmptyPosting, DiscardEarlierVersions, etc.). The caller is supposed to run Next as needed.

[manishrjain]

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @gitlw and @martinmr)

---

ee/backup/backup.go, line 272 at r1 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

Not relevant anymore.

I'm looking at this code. toBackupList should not be in a for loop. ReadPostingList takes care of advancing the iterator to generate the posting list.

[gitlw]

@martinmr I fixed this problem in #3869 in order to unblock the 1.1 release. So closing this PR now.