app/vmselect/netstorage: prevent from disk write IO when closing temporary files

Remove temporary file before closing it in order to signal the OS that it shouldn't
store the file contents from page cache to disk when the file is closed.

Gracefully handle the case when the file cannot be removed before being closed -
in this case remove the file after closing it. This allows working on Windows.

Also remove superflouos opening of temporary file for reading - re-use already opened file handle for writing.

This is a follow-up for 9b1e002287
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4020
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/70

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 {
-		return
-	}
+	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 {
-		// tbf.r could be nil if Finalize wasn't called.
-		tbf.r.MustClose()
+			logger.Panicf("BUG: tbf.r must be nil when tbf.f!=nil")
 		}
-	fname := tbf.f.Name()
 
+		// 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)
 		}
-	// We cannot remove unclosed at non-posix filesystems, like windows
+		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 {
+		// Nothing to do
+		return
+	}
+
+	// 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.r = nil
 }

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)
 		}