chore: improve error messages and logging during shard opening#25314
chore: improve error messages and logging during shard opening#25314gwossum merged 2 commits intomaster-1.xfrom
Conversation
davidby-influx
left a comment
There was a problem hiding this comment.
This will really help us debugging in the future. Nice work. I'd like to add (or in a few cases) restore the path/file to the error messages as a little more guidance for our debugging.
tsdb/engine/tsm1/engine.go
Outdated
| if f.IsDir() && strings.HasSuffix(f.Name(), ext) { | ||
| if err := os.RemoveAll(filepath.Join(e.path, f.Name())); err != nil { | ||
| return fmt.Errorf("error removing tmp snapshot directory %q: %s", f.Name(), err) | ||
| return fmt.Errorf("Engine.cleanup: error removing tmp snapshot directory: %w", err) |
There was a problem hiding this comment.
Let's keep the file name in the message.
There was a problem hiding this comment.
I had that originally, but then I noticed that PathError contained the path so I removed the duplicate path. Upon closer inspection, these are loafers if you get a SyscallError, that doesn't include the path.
tsdb/engine/tsm1/file_store.go
Outdated
| file, err := os.OpenFile(fn, os.O_RDONLY, 0666) | ||
| if err != nil { | ||
| return fmt.Errorf("error opening file %s: %v", fn, err) | ||
| return fmt.Errorf("FileStore.Open: %w", err) |
There was a problem hiding this comment.
Let's keep the file name in the message
tsdb/engine/tsm1/file_store.go
Outdated
|
|
||
| start := time.Now() | ||
| df, err := NewTSMReader(file, f.readerOptions...) | ||
|
|
| // If we are unable to read a TSM file then log the error. | ||
| if err != nil { | ||
| file.Close() | ||
| if cerr := file.Close(); cerr != nil { |
tsdb/engine/tsm1/file_store.go
Outdated
| } else { | ||
| close(readerC) | ||
| return err | ||
| return fmt.Errorf("FileStore.Open: %w", err) |
There was a problem hiding this comment.
Let's add the directory name here.
tsdb/index.go
Outdated
| // nop, use default | ||
| } else if err != nil { | ||
| return nil, err | ||
| return nil, fmt.Errorf("NewIndex: %w", err) |
There was a problem hiding this comment.
Let's add the path in the message
No description provided.