From e1a6262302953961905e2f932c748c0f45ffa798 Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@gmail.com>
Date: Mon, 23 Nov 2020 09:55:38 +0200
Subject: [PATCH] lib/fs: replace fs.OpenReaderAt with fs.MustOpenReaderAt
All the callers for fs.OpenReaderAt expect that the file will be opened.
So it is better to log fatal error inside fs.MustOpenReaderAt instead of leaving this to the caller.
---
app/vmselect/netstorage/tmp_blocks_file.go | 5 +----
lib/fs/reader_at.go | 13 +++++++------
lib/fs/reader_at_test.go | 5 +----
lib/fs/reader_at_timing_test.go | 5 +----
lib/mergeset/part.go | 21 +++------------------
lib/storage/part.go | 18 +++---------------
6 files changed, 16 insertions(+), 51 deletions(-)
diff --git a/app/vmselect/netstorage/tmp_blocks_file.go b/app/vmselect/netstorage/tmp_blocks_file.go
index 71bd7561c1..7a16131a3b 100644
--- a/app/vmselect/netstorage/tmp_blocks_file.go
+++ b/app/vmselect/netstorage/tmp_blocks_file.go
@@ -133,10 +133,7 @@ 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, err := fs.OpenReaderAt(fname)
- if err != nil {
- logger.Panicf("FATAL: cannot open %q: %s", fname, err)
- }
+ r := fs.MustOpenReaderAt(fname)
// Hint the OS that the file is read almost sequentiallly.
// This should reduce the number of disk seeks, which is important
// for HDDs.
diff --git a/lib/fs/reader_at.go b/lib/fs/reader_at.go
index 175292cc07..00b05d24ea 100644
--- a/lib/fs/reader_at.go
+++ b/lib/fs/reader_at.go
@@ -158,13 +158,13 @@ func (r *ReaderAt) MustFadviseSequentialRead(prefetch bool) {
}
}
-// OpenReaderAt opens ReaderAt for reading from filename.
+// MustOpenReaderAt opens ReaderAt for reading from filename.
//
// MustClose must be called on the returned ReaderAt when it is no longer needed.
-func OpenReaderAt(path string) (*ReaderAt, error) {
+func MustOpenReaderAt(path string) *ReaderAt {
f, err := os.Open(path)
if err != nil {
- return nil, fmt.Errorf("cannot open file %q for reader: %w", path, err)
+ logger.Panicf("FATAL: cannot open file %q for reading: %s", path, err)
}
var r ReaderAt
r.f = f
@@ -172,7 +172,8 @@ func OpenReaderAt(path string) (*ReaderAt, error) {
if !*disableMmap {
fi, err := f.Stat()
if err != nil {
- return nil, fmt.Errorf("error in stat: %w", err)
+ MustClose(f)
+ logger.Panicf("FATAL: error in fstat(%q): %s", path, err)
}
size := fi.Size()
bm := &pageCacheBitmap{
@@ -188,12 +189,12 @@ func OpenReaderAt(path string) (*ReaderAt, error) {
data, err := mmapFile(f, size)
if err != nil {
MustClose(f)
- return nil, fmt.Errorf("cannot init reader for %q: %w", path, err)
+ logger.Panicf("FATAL: cannot mmap %q: %s", path, err)
}
r.mmapData = data
}
readersCount.Inc()
- return &r, nil
+ return &r
}
func pageCacheBitmapCleaner(pcbm *atomic.Value, stopCh <-chan struct{}) {
diff --git a/lib/fs/reader_at_test.go b/lib/fs/reader_at_test.go
index 10d84c7388..4747030e78 100644
--- a/lib/fs/reader_at_test.go
+++ b/lib/fs/reader_at_test.go
@@ -22,10 +22,7 @@ func testReaderAt(t *testing.T, bufSize int) {
t.Fatalf("cannot create %q: %s", path, err)
}
defer MustRemoveAll(path)
- r, err := OpenReaderAt(path)
- if err != nil {
- t.Fatalf("error in OpenReaderAt(%q): %s", path, err)
- }
+ r := MustOpenReaderAt(path)
defer r.MustClose()
buf := make([]byte, bufSize)
diff --git a/lib/fs/reader_at_timing_test.go b/lib/fs/reader_at_timing_test.go
index 84eb1e65af..87b012e973 100644
--- a/lib/fs/reader_at_timing_test.go
+++ b/lib/fs/reader_at_timing_test.go
@@ -29,10 +29,7 @@ func benchmarkReaderAtMustReadAt(b *testing.B, isMmap bool) {
b.Fatalf("cannot create %q: %s", path, err)
}
defer MustRemoveAll(path)
- r, err := OpenReaderAt(path)
- if err != nil {
- b.Fatalf("error in OpenReaderAt(%q): %s", path, err)
- }
+ r := MustOpenReaderAt(path)
defer r.MustClose()
b.ResetTimer()
diff --git a/lib/mergeset/part.go b/lib/mergeset/part.go
index 86bbc5bb8b..0ead8d97c8 100644
--- a/lib/mergeset/part.go
+++ b/lib/mergeset/part.go
@@ -78,30 +78,15 @@ func openFilePart(path string) (*part, error) {
metaindexSize := fs.MustFileSize(metaindexPath)
indexPath := path + "/index.bin"
- indexFile, err := fs.OpenReaderAt(indexPath)
- if err != nil {
- metaindexFile.MustClose()
- return nil, fmt.Errorf("cannot open %q: %w", indexPath, err)
- }
+ indexFile := fs.MustOpenReaderAt(indexPath)
indexSize := fs.MustFileSize(indexPath)
itemsPath := path + "/items.bin"
- itemsFile, err := fs.OpenReaderAt(itemsPath)
- if err != nil {
- metaindexFile.MustClose()
- indexFile.MustClose()
- return nil, fmt.Errorf("cannot open %q: %w", itemsPath, err)
- }
+ itemsFile := fs.MustOpenReaderAt(itemsPath)
itemsSize := fs.MustFileSize(itemsPath)
lensPath := path + "/lens.bin"
- lensFile, err := fs.OpenReaderAt(lensPath)
- if err != nil {
- metaindexFile.MustClose()
- indexFile.MustClose()
- itemsFile.MustClose()
- return nil, fmt.Errorf("cannot open %q: %w", lensPath, err)
- }
+ lensFile := fs.MustOpenReaderAt(lensPath)
lensSize := fs.MustFileSize(lensPath)
size := metaindexSize + indexSize + itemsSize + lensSize
diff --git a/lib/storage/part.go b/lib/storage/part.go
index 2eb62b1803..1fcd969d57 100644
--- a/lib/storage/part.go
+++ b/lib/storage/part.go
@@ -60,27 +60,15 @@ func openFilePart(path string) (*part, error) {
}
timestampsPath := path + "/timestamps.bin"
- timestampsFile, err := fs.OpenReaderAt(timestampsPath)
- if err != nil {
- return nil, fmt.Errorf("cannot open timestamps file: %w", err)
- }
+ timestampsFile := fs.MustOpenReaderAt(timestampsPath)
timestampsSize := fs.MustFileSize(timestampsPath)
valuesPath := path + "/values.bin"
- valuesFile, err := fs.OpenReaderAt(valuesPath)
- if err != nil {
- timestampsFile.MustClose()
- return nil, fmt.Errorf("cannot open values file: %w", err)
- }
+ valuesFile := fs.MustOpenReaderAt(valuesPath)
valuesSize := fs.MustFileSize(valuesPath)
indexPath := path + "/index.bin"
- indexFile, err := fs.OpenReaderAt(indexPath)
- if err != nil {
- timestampsFile.MustClose()
- valuesFile.MustClose()
- return nil, fmt.Errorf("cannot open index file: %w", err)
- }
+ indexFile := fs.MustOpenReaderAt(indexPath)
indexSize := fs.MustFileSize(indexPath)
metaindexPath := path + "/metaindex.bin"