Badger: 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 (https://github.com/ipfs/go-ds-badger/issues/86).

When estimating the size of an entry:

https://github.com/dgraph-io/badger/blob/8a93a41fef4ba548fc241157164282cb6ace892f/structs.go#L153-L158

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:

https://github.com/dgraph-io/badger/blob/617ed7c7db9d618b6511adfff5d22dcde2233049/value.go#L566-L576

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 https://github.com/dgraph-io/badger/pull/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 https://github.com/dgraph-io/badger/pull/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.