internal/manual: track manually-allocated memory size and expose it through metrics

[jwasinger]

Because Pebble manages memory manually, it is not clear to me how to get accurate insight into memory consumption. Is there an alternative solution than the changes introduced in this PR?

[cockroach-teamcity]

This change is 

[jwasinger]

Hey @jbowens @nicktrav @bananabrick , we're impressed with pebble and want to adopt it as the db backing https://github.com/ethereum/go-ethereum .

Exposing this metric would be nice to have for us. Any feedback you have on the changes here would be greatly appreciated if you get the time.

Thanks,
Jared

[nicktrav]

@jwasinger - thanks for the nudge, and the kind words. Adding some folks for thoughts on this - we have a number of metrics already, so it's possible there's a way to get this already.

cc: @cockroachdb/storage

[jbowens]

Hi @jwasinger — sorry for not looking at this earlier 😞

Some background: Pebble manually allocates memory for two purposes: memtables and the block cache.

When a DB is opened, you can configure the maximum size of the block cache by setting Options.Cache. When Pebble needs to allocate memory for a new memtable, it "reserves" memory from the memory allocated to the block cache, effectively shrinking the size of the block cache. This means the size you provide to cache.NewCache is (almost) an upper bound on the memory manually allocated by Pebble. I think it's not a true upper bound because memtables are allowed to reserve more memory than is allocated to block cache. I think the true upper bound is Max(Options.MemTableSize*Options.MemTableStopWritesThreshold, CacheSize).

For getting the current manually-allocated total, I think it can be derived from existing metrics. Metrics.BlockCache.Size provides the total manually-allocated bytes in the block cache. This sum does not include memory from the memtables. Adding Metrics.MemTable.Size and Metrics.MemTable.ZombieSize gives the total manually-allocated memory.

ManuallyAllocated = Metrics.BlockCache.Size + Metrics.MemTable.Size + Metrics.MemTable.ZombieSize

There is one caveat. If a very large batch is committed and the batch's size is more than half the memtable size, Pebble will queue it to be flushed rather than copying it into the memtable, and these batches will be included in the memtable size metrics. These batches are Go-allocated, and so in these cases the above calculation is an overestimate.

Is this enough for your needs? If not, maybe we can add a new metric that differentiates between large batches and manually-allocated memtables.

Reviewable status: 0 of 4 files reviewed, all discussions resolved

[jwasinger]

Thanks for the suggestion @jbowens . I tried it out and it seems to under-count slightly. e.g. I have a go-ethereum node running right now and this PR reports 2.37gb allocated vs 2.15gb from the heuristic you are suggesting.

So I think from our perspective, it would be nice to have a metric for the exact amount of manually-allocated memory instead of relying on a heuristic.

[karalabe]

Hey all,

The reason we prefer lower level - counted - metrics is because it catches bugs where a refactor slightly modifies some data structure and the "heuristic" still says it's ok, but in reality the real usage bubbles away.

The second reason is that we'd like to accurately report the total memory usage of our process and alert if it goes over some threshold. Ideally such a thing could be done via OS monitoring, but in practice since Go switched over to relinquishing unused memory via OS hints instead of actual "frees", the OS counters are useless as they cannot differentiate between memory held-and-used and memory-held-but-allowed-to-be-taken. We can still get an accurate RAM usage from the Go runtime - and have been doing that ever since - but once something goes around the Go runtime, we need to reel it in somehow into our metrics system.

Hope this makes it a bit clearer why we're looking for this.

[holiman]

Hi,

Gentle ping on this, we (go-ethereum) would love to get this merged. As @karalabe pointed out, we'd much rather rely on the actual number rather than heuristics.

The overhead incurred by this PR should be minimal, since atomic operations on uint64 are actually overhead-free on 64-bit platforms, as far as I understand.

[MariusVanDerWijden]

Another gentle ping on this. It is actually the last piece missing of us shipping Pebble support in go-ethereum. Would be nice to have a decision if you can imagine this going in or you are categorically against this change