sideloading: 2x disk usage after a restore

[danhhz]

When restoring X bytes of data, you need 2X (plus some headroom) of available space on the cluster. This is because (until the new tables get some traffic) the raft log for the new ranges is not truncated, so an entire copy of the restored data ends up sitting in raft logs.

We fixed this once by making the raft log truncation work by both number of entries and total size, but it seems there's a regression when it comes to sideloading. Perhaps the truncation threshold logic doesn't take into account the fully hydrated command size?

[petermattis]

Is there an easy way to reproduce this without running a large restore? That is, can you give me a small test that uses sideloaded data. Shouldn't be difficult to track down where the lack of truncation is occurring.

[danhhz]

I could provide you with a 1GB restore. Anything smaller that that would probably require some work on my end to set up

[petermattis]

Is there a test I can run which generates sideloaded data?

…

On Thu, Aug 31, 2017 at 12:14 PM Daniel Harrison ***@***.***> wrote: I could provide you with a 1GB restore. Anything smaller that that would probably require some work on my end to set up — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#18077 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AF6f9-nto8K4Mw43Yuv4Jz1EhHTq1kL3ks5sdtu2gaJpZM4PJDWr> .

[danhhz]

TestDBAddSSTable

[petermattis]

This is the code which adjusts the Raft log size when appending entries:

	if len(rd.Entries) > 0 {
		// All of the entries are appended to distinct keys, returning a new
		// last index.
		thinEntries, err := r.maybeSideloadEntriesRaftMuLocked(ctx, rd.Entries)
		if err != nil {
			return stats, err
		}
		if lastIndex, lastTerm, raftLogSize, err = r.append(
			ctx, writer, lastIndex, lastTerm, raftLogSize, thinEntries,
		); err != nil {
			return stats, err
		}
	}

Because the size is adjust when appending the thinEntries we don't account for the size of the side-loaded data. It seems easy to perform this accounting in maybeSideloadEntriesRaftMuLocked. We'd also have to fix Replica.applySnapshot which has a similar bit of code. @tschottdorf Am I missing anything here? I'm guessing this was a simple oversight.

[tbg]

No, just an oversight. Thanks for cleaning up!

On Thu, Aug 31, 2017, 14:54 Peter Mattis ***@***.***> wrote: This <https://github.com/cockroachdb/cockroach/blob/master/pkg/storage/replica.go#L3273-L3285> is the code which adjusts the Raft log size when appending entries: if len(rd.Entries) > 0 { // All of the entries are appended to distinct keys, returning a new // last index. thinEntries, err := r.maybeSideloadEntriesRaftMuLocked(ctx, rd.Entries) if err != nil { return stats, err } if lastIndex, lastTerm, raftLogSize, err = r.append( ctx, writer, lastIndex, lastTerm, raftLogSize, thinEntries, ); err != nil { return stats, err } } Because the size is adjust when appending the thinEntries we don't account for the size of the side-loaded data. It seems easy to perform this accounting in maybeSideloadEntriesRaftMuLocked. We'd also have to fix Replica.applySnapshot which has a similar bit of code. @tschottdorf <https://github.com/tschottdorf> Am I missing anything here? I'm guessing this was a simple oversight. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#18077 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE135HFw2liDFBfaHrhRjQ92Xj020Eu9ks5sdwFzgaJpZM4PJDWr> .

--

…

-- Tobias