diff --git a/app/vmselect/netstorage/tmp_blocks_file.go b/app/vmselect/netstorage/tmp_blocks_file.go index ba3e60de3..78288d538 100644 --- a/app/vmselect/netstorage/tmp_blocks_file.go +++ b/app/vmselect/netstorage/tmp_blocks_file.go @@ -136,21 +136,25 @@ func (tbf *tmpBlocksFile) Finalize() error { return fmt.Errorf("cannot write the remaining %d bytes to %q: %w", len(tbf.buf), fname, err) } tbf.buf = tbf.buf[:0] - r := fs.MustOpenReaderAt(fname) + r := fs.NewReaderAt(tbf.f) + // Hint the OS that the file is read almost sequentiallly. // This should reduce the number of disk seeks, which is important // for HDDs. r.MustFadviseSequentialRead(true) + // Collect local stats in order to improve performance on systems with big number of CPU cores. // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3966 r.SetUseLocalStats() + tbf.r = r + tbf.f = nil return nil } func (tbf *tmpBlocksFile) MustReadBlockRefAt(partRef storage.PartRef, addr tmpBlockAddr) storage.BlockRef { var buf []byte - if tbf.f == nil { + if tbf.r == nil { buf = tbf.buf[addr.offset : addr.offset+uint64(addr.size)] } else { bb := tmpBufPool.Get() @@ -169,21 +173,45 @@ func (tbf *tmpBlocksFile) MustReadBlockRefAt(partRef storage.PartRef, addr tmpBl var tmpBufPool bytesutil.ByteBufferPool func (tbf *tmpBlocksFile) MustClose() { - if tbf.f == nil { + if tbf.f != nil { + // tbf.f could be non-nil if Finalize wasn't called. + // In this case tbf.r must be nil. + if tbf.r != nil { + logger.Panicf("BUG: tbf.r must be nil when tbf.f!=nil") + } + + // Try removing the file before closing it in order to prevent from flushing the in-memory data + // from page cache to the disk and save disk write IO. This may fail on non-posix systems such as Windows. + // Gracefully handle this case by attempting to remove the file after closing it. + fname := tbf.f.Name() + errRemove := os.Remove(fname) + if err := tbf.f.Close(); err != nil { + logger.Panicf("FATAL: cannot close %q: %s", fname, err) + } + if errRemove != nil { + if err := os.Remove(fname); err != nil { + logger.Panicf("FATAL: cannot remove %q: %s", fname, err) + } + } + tbf.f = nil return } - if tbf.r != nil { - // tbf.r could be nil if Finalize wasn't called. - tbf.r.MustClose() - } - fname := tbf.f.Name() - if err := tbf.f.Close(); err != nil { - logger.Panicf("FATAL: cannot close %q: %s", fname, err) + if tbf.r == nil { + // Nothing to do + return } - // We cannot remove unclosed at non-posix filesystems, like windows - if err := os.Remove(fname); err != nil { - logger.Panicf("FATAL: cannot remove %q: %s", fname, err) + + // Try removing the file before closing it in order to prevent from flushing the in-memory data + // from page cache to the disk and save disk write IO. This may fail on non-posix systems such as Windows. + // Gracefully handle this case by attempting to remove the file after closing it. + fname := tbf.r.Path() + errRemove := os.Remove(fname) + tbf.r.MustClose() + if errRemove != nil { + if err := os.Remove(fname); err != nil { + logger.Panicf("FATAL: cannot remove %q: %s", fname, err) + } } - tbf.f = nil + tbf.r = nil } diff --git a/lib/fs/reader_at.go b/lib/fs/reader_at.go index 1b170f9e2..3fdd85bd1 100644 --- a/lib/fs/reader_at.go +++ b/lib/fs/reader_at.go @@ -121,7 +121,7 @@ func (r *ReaderAt) MustFadviseSequentialRead(prefetch bool) { } } -// MustOpenReaderAt opens ReaderAt for reading from filename. +// MustOpenReaderAt opens ReaderAt for reading from the file located at path. // // MustClose must be called on the returned ReaderAt when it is no longer needed. func MustOpenReaderAt(path string) *ReaderAt { @@ -129,17 +129,28 @@ func MustOpenReaderAt(path string) *ReaderAt { if err != nil { logger.Panicf("FATAL: cannot open file for reading: %s", err) } + return NewReaderAt(f) +} + +// NewReaderAt returns ReaderAt for reading from f. +// +// NewReaderAt takes ownership for f, so it shouldn't be closed by the caller. +// +// MustClose must be called on the returned ReaderAt when it is no longer needed. +func NewReaderAt(f *os.File) *ReaderAt { var r ReaderAt r.f = f if !*disableMmap { fi, err := f.Stat() if err != nil { + path := f.Name() MustClose(f) logger.Panicf("FATAL: error in fstat(%q): %s", path, err) } size := fi.Size() data, err := mmapFile(f, size) if err != nil { + path := f.Name() MustClose(f) logger.Panicf("FATAL: cannot mmap %q: %s", path, err) }