Use ReadTier::kMemtableTier in read-before-write for max_successive_merges

[duarten]

Currently, max_successive_merges performs a read-before-write from disk to replace the current merge value in the memtable with a put. That read is blocking, which stalls the current thread. It would be better if max_successive_merges performed the read only within the memtable tier, replacing the current set of merge operands with the result of partially merging them. This likely requires introducing a new ValueType, and maybe a new toggle.

[ajkr]

cc @benclay - would this be of use to your team to avoid the occasional latency due to read I/O in the write path while still enabling max_successive_merges? Sorry if there was another resolution; I can't remember now.

Somehow FB doesn't have much in the way of merge-heavy latency-sensitive use cases, so this feature would be hard to prioritize. But it would be a great feature to have.

I think the new ValueType could be memtable only and not persisted anywhere. The new ValueType would indicate the value contains the merged result for the memtable up to that point. Then only memtable reads need to know how to interpret it and there's no compatibility concerns.

[benclay]

Yes we could definitely make use of it. We've gotten around it by just disabling max_sucessive_merges on the clusters that have that read anti-pattern. Agree that in-memory should be sufficient based on our past discussion.

…

-Ben

On Tue, Jul 13, 2021, 10:05 PM Andrew Kryczka ***@***.***> wrote: cc @benclay <https://github.com/benclay> - would this be of use to your team to avoid the occasional latency due to read I/O in the write path while still enabling max_successive_merges? Sorry if there was another resolution; I can't remember now. Somehow FB doesn't have much in the way of merge-heavy latency-sensitive use cases, so this feature would be hard to prioritize. But it would be a great feature to have. I think the new ValueType could be memtable only and not persisted anywhere. The new ValueType would indicate the value contains the merged result for the memtable up to that point. Then only MemTableIterator needs to know how to read it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8516 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB2BMMW7G3Y6PRIVNCDHF3DTXTWF5ANCNFSM5AHVNGWA> .

[ajkr]

We finally ran into this. #12365 should improve things although the solution discussed in this issue would be even better.