Ensure bitValuePointer flag is cleared for LSM entry values written to LSM

[ou05020]

When restoring a backup that was written with a lower ValueThreshold to a DB instance with a higher ValueThreshold, some entry values originally written to the value log in the backup DB will be written to the LSM along with the key when restored to the new DB.

The meta field for those entries is not updated to reflect the correct value storage state. Those entries still have the bitValuePointer flag set.

That causes iterator.prefetch to fail while looking up the value for those entries.

This fix ensures the meta bitValuePointer is cleared when entry values are stored in the LSM.

The original error can be reproduced by running the included TestLSMVPClear test after reverting the line 687 db.go change to clear the bitValuePointer flag.

---

This change is 

[jarifibrahim]

Thank you for fixing this PR @ou05020 . We really appreciate it. I have one small comment. Please address it and we can merge this PR.

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @jarifibrahim, @manishrjain, and @ou05020)

---

backup_test.go, line 127 at r1 (raw file):

// with:
// 			Meta: entry.meta,
func TestLSMVPClear(t *testing.T) {

Thank you for adding a test. It's very easy to review a PR that has a test. I have written a similar test that looks simpler to me.

https://gist.github.com/jarifibrahim/aa134b1f8b35ddc1749ee24eb3198c23

I'll leave it up to you to decide which one looks simpler to you. :)

---

db.go, line 687 at r1 (raw file):

					// known to be set in write to LSM when the entry is loaded from a backup
					// with lower ValueThreshold and its value was stored in the value log.
					Meta:      entry.meta &^ bitValuePointer,

This will fix the issue but the actual issue is on line https://github.com/dgraph-io/badger/blob/5d19cc727d879fbab828b5299c47d53d23c3af7d/backup.go#L74

This is where we store the bits in the backup. Ideally, we should remove all the unnecessary bits from here so when restore happens, the bits will be stored correctly.
what do you think?

[ou05020]

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @ashish-goswami, @jarifibrahim, and @manishrjain)

---

backup_test.go, line 127 at r1 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

Thank you for adding a test. It's very easy to review a PR that has a test. I have written a similar test that looks simpler to me.

https://gist.github.com/jarifibrahim/aa134b1f8b35ddc1749ee24eb3198c23

I'll leave it up to you to decide which one looks simpler to you. :)

Thank you, I like your test better.

---

db.go, line 687 at r1 (raw file):

Previously, jarifibrahim (Ibrahim Jarif) wrote…

This will fix the issue but the actual issue is on line https://github.com/dgraph-io/badger/blob/5d19cc727d879fbab828b5299c47d53d23c3af7d/backup.go#L74

This is where we store the bits in the backup. Ideally, we should remove all the unnecessary bits from here so when restore happens, the bits will be stored correctly.
what do you think?

I originally modified backup.go to clear the value pointer bit. However, that did not address restoring from backups already created with the bit set.

Explicitly setting/clearing the value pointer bit based on how it will be stored at the time it is written is safest.

It resolves restoring from older backups with the problem and it prevents future errors that may occur if entries are copied/reused but the value is updated in such a way the ValueThreshold evaluation is different from the previous value.

[manishrjain]

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ashish-goswami and @jarifibrahim)

[cedricfung]

Will this fix #1139, without RunValueLogGC our data are increasing so fast.

[jarifibrahim]

@cedricfung, the issue with #1139 was that you had some entries with invalid bitPointerValue set. This PR fixes the bits on invalid entries.

Did you take a backup and restore it with different ValueThreshold value?

[jarifibrahim]

Thank you for fixing this @ou05020 🎉

[cedricfung]

@jarifibrahim sorry I don't understand this, what could I do to fix this?

[jarifibrahim]

@cedricfung Take a backup of your badger directory and restore it. (Keep the original one around). Do you run badger with default options?