feat: WIP defer'ing compaction decrement #26273
Conversation
| p.levelCompacting[0] = false | ||
| p.mu.Unlock() | ||
| p.Compact() | ||
| defer func() { |
There was a problem hiding this comment.
The defer should go at the top of the lambda, otherwise this change won't actually make any difference. Even then, it would only make a difference if p.compactLogFile panics.
| p.levelCompacting[0] = false | ||
| p.mu.Unlock() | ||
| p.Compact() | ||
| defer func() { |
There was a problem hiding this comment.
The defer has to be called before the p.compactLogFile
|
|
||
| // Check for new compactions | ||
| p.Compact() | ||
| defer func() { |
There was a problem hiding this comment.
the defer has to be called before the p.compactToLevel
|
Overall, I'm not sure the What's the scenario where |
| p.currentCompactionN-- | ||
| p.levelCompacting[0] = false | ||
| p.mu.Unlock() | ||
| p.Compact() |
There was a problem hiding this comment.
The p.Compact call here leads to indirect tail-call recursion. Putting the call in a defer will add another level to the stack. Depending on how deep the recursion goes, adding this extra stack frame every time may cause problems. Do we think p.compactLogFile panics?
There was a problem hiding this comment.
More specifically, do we think p.compactLogFile panics, and then that is recovered higher up in the stack? Otherwise, I don't see this change making any difference.
No description provided.