fix: ignore empty index error deleting last measurement (#25037)#27086
fix: ignore empty index error deleting last measurement (#25037)#27086davidby-influx merged 2 commits intomain-2.xfrom
Conversation
| return fmt.Errorf("failed to write index to TSM file: %w", err) | ||
| // Write index & close. | ||
| // It is okay to have no index values if no block was written | ||
| if err := w.WriteIndex(); err != nil && !(hasData || errors.Is(err, tsm1.ErrNoValues)) { |
There was a problem hiding this comment.
It seems like this will discard errors as long as hasData = true. For instance, if hasData = true and w.WriteIndex returns an out of space error (syscall.ENOSPC):
err != nil && !(hasData || errors.Is(err, tsm1.ErrNoValues))
syscall.ENOSPC != nil && !(true || errors.Is(syscall.ENOSPC, tsm1.ErrNoValues))
true && !(true || false)
true && !true
true && false
false
So the conditional will evaluate to false and we will ignore the syscall.ENOSPC returned by w.WriteIndex.
Am I missing something, or did I get my logic incorrect?
There was a problem hiding this comment.
Let me channel the ghost of George Boole tomorrow....
There was a problem hiding this comment.
Are we missing a negation of hasData like this?
err != nil && !(!hasData || errors.Is(err, tsm1.ErrNoValues))
gwossum
left a comment
There was a problem hiding this comment.
I've got a question about some logic. I approved the original PR this is based on, but now I have some doubt about the correctness.
|
Claude thoughts:
|
See discussion in #27086
An empty index is appropriate when deleting the last measurement. Also clean up error handling, avoid
duplicate calls to Close.
closes #9929
(cherry picked from commit b09e4b7)
Fixes #27037