From 2a8395be0503c934073ccc06c82a4ab6e37f3889 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Thu, 13 Apr 2023 21:33:15 -0700 Subject: [PATCH] lib/fs: replace WriteFileAndSync with MustWriteAndSync When WriteFileAndSync fails, then the caller eventually logs the error message and exits. The error message returned by WriteFileAndSync already contains the path to the file, which couldn't be created. This information alongside the call stack is enough for debugging the issue. So just use log.Panicf("FATAL: ...") inside MustWriteAndSync(). This simplifies error handling at caller side a bit. --- lib/fs/fs.go | 13 +++++-------- lib/mergeset/inmemory_part.go | 25 ++++++++++--------------- lib/mergeset/part_header.go | 7 ++----- lib/mergeset/table.go | 4 +--- lib/storage/inmemory_part.go | 25 ++++++++++--------------- lib/storage/part_header.go | 7 ++----- lib/storage/partition.go | 4 +--- 7 files changed, 31 insertions(+), 54 deletions(-) diff --git a/lib/fs/fs.go b/lib/fs/fs.go index f8e90c104..d343f1fb5 100644 --- a/lib/fs/fs.go +++ b/lib/fs/fs.go @@ -25,7 +25,7 @@ func MustSyncPath(path string) { mustSyncPath(path) } -// WriteFileAndSync writes data to the file at path and then calls fsync on the created file. +// MustWriteFileAndSync writes data to the file at path and then calls fsync on the created file. // // The fsync guarantees that the written data survives hardware reset after successful call. // @@ -33,20 +33,19 @@ func MustSyncPath(path string) { // in the middle of the write. // Use WriteFileAtomically if the file at the path must be either written in full // or not written at all on app crash in the middle of the write. -func WriteFileAndSync(path string, data []byte) error { +func MustWriteFileAndSync(path string, data []byte) { f, err := filestream.Create(path, false) if err != nil { - return err + logger.Panicf("FATAL: cannot create file: %s", err) } if _, err := f.Write(data); err != nil { f.MustClose() // Do not call MustRemoveAll(path), so the user could inspect // the file contents during investigation of the issue. - return fmt.Errorf("cannot write %d bytes to %q: %w", len(data), path, err) + logger.Panicf("FATAL: cannot write %d bytes to %q: %s", len(data), path, err) } // Sync and close the file. f.MustClose() - return nil } // WriteFileAtomically atomically writes data to the given file path. @@ -70,9 +69,7 @@ func WriteFileAtomically(path string, data []byte, canOverwrite bool) error { // Write data to a temporary file. n := atomic.AddUint64(&tmpFileNum, 1) tmpPath := fmt.Sprintf("%s.tmp.%d", path, n) - if err := WriteFileAndSync(tmpPath, data); err != nil { - return fmt.Errorf("cannot write data to temporary file: %w", err) - } + MustWriteFileAndSync(tmpPath, data) // Atomically move the temporary file from tmpPath to path. if err := os.Rename(tmpPath, path); err != nil { diff --git a/lib/mergeset/inmemory_part.go b/lib/mergeset/inmemory_part.go index a638a410d..500bc1883 100644 --- a/lib/mergeset/inmemory_part.go +++ b/lib/mergeset/inmemory_part.go @@ -38,24 +38,19 @@ func (mp *inmemoryPart) StoreToDisk(path string) error { return fmt.Errorf("cannot create directory %q: %w", path, err) } metaindexPath := filepath.Join(path, metaindexFilename) - if err := fs.WriteFileAndSync(metaindexPath, mp.metaindexData.B); err != nil { - return fmt.Errorf("cannot store metaindex: %w", err) - } + fs.MustWriteFileAndSync(metaindexPath, mp.metaindexData.B) + indexPath := filepath.Join(path, indexFilename) - if err := fs.WriteFileAndSync(indexPath, mp.indexData.B); err != nil { - return fmt.Errorf("cannot store index: %w", err) - } + fs.MustWriteFileAndSync(indexPath, mp.indexData.B) + itemsPath := filepath.Join(path, itemsFilename) - if err := fs.WriteFileAndSync(itemsPath, mp.itemsData.B); err != nil { - return fmt.Errorf("cannot store items: %w", err) - } + fs.MustWriteFileAndSync(itemsPath, mp.itemsData.B) + lensPath := filepath.Join(path, lensFilename) - if err := fs.WriteFileAndSync(lensPath, mp.lensData.B); err != nil { - return fmt.Errorf("cannot store lens: %w", err) - } - if err := mp.ph.WriteMetadata(path); err != nil { - return fmt.Errorf("cannot store metadata: %w", err) - } + fs.MustWriteFileAndSync(lensPath, mp.lensData.B) + + mp.ph.MustWriteMetadata(path) + fs.MustSyncPath(path) // Do not sync parent directory - it must be synced by the caller. return nil diff --git a/lib/mergeset/part_header.go b/lib/mergeset/part_header.go index 1f46907ec..c77de65c1 100644 --- a/lib/mergeset/part_header.go +++ b/lib/mergeset/part_header.go @@ -113,7 +113,7 @@ func (ph *partHeader) ReadMetadata(partPath string) error { return nil } -func (ph *partHeader) WriteMetadata(partPath string) error { +func (ph *partHeader) MustWriteMetadata(partPath string) { phj := &partHeaderJSON{ ItemsCount: ph.itemsCount, BlocksCount: ph.blocksCount, @@ -128,8 +128,5 @@ func (ph *partHeader) WriteMetadata(partPath string) error { // There is no need in calling fs.WriteFileAtomically() here, // since the file is created only once during part creatinng // and the part directory is synced aftewards. - if err := fs.WriteFileAndSync(metadataPath, metadata); err != nil { - return fmt.Errorf("cannot create %q: %w", metadataPath, err) - } - return nil + fs.MustWriteFileAndSync(metadataPath, metadata) } diff --git a/lib/mergeset/table.go b/lib/mergeset/table.go index fdbe09849..eadb40de0 100644 --- a/lib/mergeset/table.go +++ b/lib/mergeset/table.go @@ -1245,9 +1245,7 @@ func (tb *Table) mergePartsInternal(dstPartPath string, bsw *blockStreamWriter, return nil, fmt.Errorf("cannot merge %d parts to %s: %w", len(bsrs), dstPartPath, err) } if dstPartPath != "" { - if err := ph.WriteMetadata(dstPartPath); err != nil { - logger.Panicf("FATAL: cannot write metadata to %s: %s", dstPartPath, err) - } + ph.MustWriteMetadata(dstPartPath) } return &ph, nil } diff --git a/lib/storage/inmemory_part.go b/lib/storage/inmemory_part.go index a8c6c3543..56719afa7 100644 --- a/lib/storage/inmemory_part.go +++ b/lib/storage/inmemory_part.go @@ -41,24 +41,19 @@ func (mp *inmemoryPart) StoreToDisk(path string) error { return fmt.Errorf("cannot create directory %q: %w", path, err) } timestampsPath := filepath.Join(path, timestampsFilename) - if err := fs.WriteFileAndSync(timestampsPath, mp.timestampsData.B); err != nil { - return fmt.Errorf("cannot store timestamps: %w", err) - } + fs.MustWriteFileAndSync(timestampsPath, mp.timestampsData.B) + valuesPath := filepath.Join(path, valuesFilename) - if err := fs.WriteFileAndSync(valuesPath, mp.valuesData.B); err != nil { - return fmt.Errorf("cannot store values: %w", err) - } + fs.MustWriteFileAndSync(valuesPath, mp.valuesData.B) + indexPath := filepath.Join(path, indexFilename) - if err := fs.WriteFileAndSync(indexPath, mp.indexData.B); err != nil { - return fmt.Errorf("cannot store index: %w", err) - } + fs.MustWriteFileAndSync(indexPath, mp.indexData.B) + metaindexPath := filepath.Join(path, metaindexFilename) - if err := fs.WriteFileAndSync(metaindexPath, mp.metaindexData.B); err != nil { - return fmt.Errorf("cannot store metaindex: %w", err) - } - if err := mp.ph.WriteMetadata(path); err != nil { - return fmt.Errorf("cannot store metadata: %w", err) - } + fs.MustWriteFileAndSync(metaindexPath, mp.metaindexData.B) + + mp.ph.MustWriteMetadata(path) + fs.MustSyncPath(path) // Do not sync parent directory - it must be synced by the caller. return nil diff --git a/lib/storage/part_header.go b/lib/storage/part_header.go index 40b17237a..7da20e60c 100644 --- a/lib/storage/part_header.go +++ b/lib/storage/part_header.go @@ -165,7 +165,7 @@ func (ph *partHeader) ReadMetadata(partPath string) error { return nil } -func (ph *partHeader) WriteMetadata(partPath string) error { +func (ph *partHeader) MustWriteMetadata(partPath string) { metadata, err := json.Marshal(ph) if err != nil { logger.Panicf("BUG: cannot marshal partHeader metadata: %s", err) @@ -174,8 +174,5 @@ func (ph *partHeader) WriteMetadata(partPath string) error { // There is no need in calling fs.WriteFileAtomically() here, // since the file is created only once during part creatinng // and the part directory is synced aftewards. - if err := fs.WriteFileAndSync(metadataPath, metadata); err != nil { - return fmt.Errorf("cannot create %q: %w", metadataPath, err) - } - return nil + fs.MustWriteFileAndSync(metadataPath, metadata) } diff --git a/lib/storage/partition.go b/lib/storage/partition.go index 1f99c901f..c7f5fa2ff 100644 --- a/lib/storage/partition.go +++ b/lib/storage/partition.go @@ -1465,9 +1465,7 @@ func (pt *partition) mergePartsInternal(dstPartPath string, bsw *blockStreamWrit } if dstPartPath != "" { ph.MinDedupInterval = GetDedupInterval() - if err := ph.WriteMetadata(dstPartPath); err != nil { - logger.Panicf("FATAL: cannot store metadata to %s: %s", dstPartPath, err) - } + ph.MustWriteMetadata(dstPartPath) } return &ph, nil }