Add Checksum to value log entries

[peterstace]

Changes:

• There is now a 32 bit CRC checksum at the end of each value log entry. So the format is now [header][key][value][crc32].
• It's checked during value log replay. If an mismatch is found, the value log is truncated at the end of the last good entry.
• It's also checked during a normal value log read.
• When doing the value log replay, we also truncate the log if a partial entry is found (i.e. we reach EOF while in the middle of an entry).

---

This change is 

[srh]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

value.go, line 150 at r1 (raw file):

		if err = read(tee, hbuf[:]); err != nil {
			if err == io.EOF {
				break

This call to the local function read() could read a few bytes and then return with io.EOF. The result is that we have to truncate the file. (And the bug is that we don't.) We could use io.ReadFull instead, or we could always truncate, to address this.

---

value_test.go, line 277 at r1 (raw file):

}

func TestPatialAppendToValueLog(t *testing.T) {

Partial

---

Comments from Reviewable

[peterstace]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

value.go, line 150 at r1 (raw file):

Previously, srh (Sam Hughes) wrote…

This call to the local function read() could read a few bytes and then return with io.EOF. The result is that we have to truncate the file. (And the bug is that we don't.) We could use io.ReadFull instead, or we could always truncate, to address this.

Fixed. I just got rid of the read function entirely handle the io.EOF and io.UnexpectedEOF errors appropriately in each case (only different for the header).

---

value_test.go, line 277 at r1 (raw file):

Previously, srh (Sam Hughes) wrote…

Partial

Fixed

---

Comments from Reviewable

[srh]

Everything else looks correct to my eyes. My main other concern is performance.

The general question is how much should we care -- disks are slow, right? I'm not so sure about the future though, what with NVM devices.

Going by this:

https://golang.org/src/hash/crc32/crc32_amd64.go

It looks like Golang supports the amd64 CRC32 instruction if we use Castagnoli instead of Koopman. This would make it about as fast as using Adler32 or Fletcher checksums on that platform.

---

Review status: 0 of 2 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[peterstace]

The whole code path for Castagnoli seems more specialised (short cuts in a few places), so using that instead.

---

Review status: 0 of 2 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[srh]

---

Reviewed 1 of 2 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[manishrjain]

---

Reviewed 1 of 2 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

value.go, line 107 at r3 (raw file):

var errStop = errors.New("Stop iteration")

Remove vertical space here.

---

value.go, line 194 at r3 (raw file):

		var vp valuePointer

		recordOffset += uint32(hlen + kl + vl + entryHashSize)

Duplication here. Just make sure that there's no special reasoning why they should be two separate calculations.

---

value.go, line 209 at r3 (raw file):

	if truncate {
		f.Lock()

No need for lock acquisition.

---

Comments from Reviewable

[peterstace]

Review status: 1 of 2 files reviewed at latest revision, 3 unresolved discussions.

---

value.go, line 107 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Remove vertical space here.

Done.

---

value.go, line 194 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Duplication here. Just make sure that there's no special reasoning why they should be two separate calculations.

Done.

---

Comments from Reviewable

[peterstace]

Review status: 1 of 2 files reviewed at latest revision, 3 unresolved discussions.

---

value.go, line 209 at r3 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

No need for lock acquisition.

Done.

---

Comments from Reviewable