Value log rewrites load the entire value log into memory

[Stebalien]

What version of Go are you using (go version)?

$ go version
go version go1.13.8 linux/amd64

What version of Badger are you using?

1.6.1

Does this issue reproduce with the latest master?

I haven't tried but the code looks the same.

Description

When rewriting a value log (e.g., on garbage collection), badger will end up loading the entire value log into memory unless there are enough small values to exceed the batch count (ipfs/go-ds-badger#86).

When estimating the size of an entry:

badger/structs.go

Lines 153 to 158 in 8a93a41

	func (e *Entry) estimateSize(threshold int) int {
	if len(e.Value) < threshold {
	return len(e.Key) + len(e.Value) + 2 // Meta, UserMeta
	}
	return len(e.Key) + 12 + 2 // 12 for ValuePointer, 2 for metas.
	}

Badger only estimates the size of the entry as it exists in the LSM.

When deciding whether or not to flush, badger only looks at this estimated size, not the size of the value itself:

badger/value.go

Lines 566 to 576 in 617ed7c

	es := int64(ne.estimateSize(vlog.opt.ValueThreshold))
	// Ensure length and size of wb is within transaction limits.
	if int64(len(wb)+1) >= vlog.opt.maxBatchCount ||
	size+es >= vlog.opt.maxBatchSize {
	tr.LazyPrintf("request has %d entries, size %d", len(wb), size)
	if err := vlog.db.batchSet(wb); err != nil {
	return err
	}
	size = 0
	wb = wb[:0]
	}

I believe the fix here is to look at the size of the value plus the LSM overhead, is that correct?

[jarifibrahim]

Yes @Stebalien. That is correct. We should consider the size of the value when considering the size of the batch.

The fix would be similar to the one in #1278

[KlotzAndrew]

Running into OOM crashes during GC, is this related? I'm trying to use badger in a low memory environment with a large disk

pprof heap screenshot, this rapidly consumes all available RAM and crashes. These are the seeing I am using:

WithLoadBloomsOnOpen(false).
WithKeepL0InMemory(false).
WithValueLogLoadingMode(options.FileIO).
WithTableLoadingMode(options.FileIO)

[jarifibrahim]

@KlotzAndrew Yes, it is the same issue. I've bumped up the priority since multiple people are seeing the same failure.

[jarifibrahim]

Hey @Stebalien @KlotzAndrew, I have a PR that should reduce the memory consumed by value log GC. Do you think you can test it #1357? I'll try to generate some data with large values and see if I can reproduce the high memory consumption.

[Stebalien]

PR looks good to me. I'll need to complete the work to support badger 2 in go-ipfs to actually test it.