fix: do not leak file handles from Compactor.write #25725
Conversation
There are a number of code paths in Compactor.write which on error can lead to leaked file handles to temporary files. This, in turn, prevents the removal of the temporary files until InfluxDB is rebooted, releasing the file handles. closes #25724
tsdb/engine/tsm1/compact.go
Outdated
| @@ -1064,11 +1064,11 @@ func (c *Compactor) writeNewFiles(generation, sequence int, src []string, iter K | |||
|
|
|||
| // We've hit the max file limit and there is more to write. Create a new file | |||
| // and continue. | |||
| if err == errMaxFileExceeded || err == ErrMaxBlocksExceeded { | |||
| if errors.Is(err, errMaxFileExceeded) || errors.Is(err, ErrMaxBlocksExceeded) { | |||
There was a problem hiding this comment.
Not sure if it matters but since you are joining errors and then checking with errors.Is if there are multiple errors joined and errMaxFileExceed or ErrMaxBlocksExceeded is in the errors this will pass the test.
See the following example where I check for a single error. The function that returns two errors combined still passes the test.
https://goplay.tools/snippet/b_YVLCYn5-j
I'm sure thats what you were expecting but just something I noticed with errors.Is
There was a problem hiding this comment.
Although the original code discards file closing errors on errMaxFileExceededor or ErrMaxBlocksExceeded, behavior that using errors.Is essentially replicates because of the feature you mention above, on reflection I think we should report and abort compaction on file closing errors, which will require an additional commit to this PR.
There are a number of code paths in Compactor.write which on error can lead to leaked file handles to temporary files. This, in turn, prevents the removal of the temporary files until InfluxDB is rebooted, releasing the file handles.
closes #25724