From 4db1858d9c4eb715b509742170864dc34c34743a Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Sun, 14 Oct 2018 18:25:46 -0700 Subject: [PATCH 01/11] wip --- src/dbnode/persist/fs/files.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/dbnode/persist/fs/files.go b/src/dbnode/persist/fs/files.go index dfabce15e0..31b4c4dced 100644 --- a/src/dbnode/persist/fs/files.go +++ b/src/dbnode/persist/fs/files.go @@ -52,6 +52,8 @@ const ( commitLogComponentPosition = 2 indexFileSetComponentPosition = 2 + + completeCheckpointFileSize = 4 // bytes ) var ( @@ -468,7 +470,7 @@ func forEachInfoFile( digestsFilePath = filesetPathFromTimeAndIndex(dir, t, volume, digestFileSuffix) infoFilePath = filesetPathFromTimeAndIndex(dir, t, volume, infoFileSuffix) } - checkpointExists, err := FileExists(checkpointFilePath) + checkpointExists, err := CompleteCheckpointFileExists(checkpointFilePath) if err != nil { continue } @@ -1035,7 +1037,7 @@ func CommitLogsDirPath(prefix string) string { func DataFileSetExistsAt(filePathPrefix string, namespace ident.ID, shard uint32, blockStart time.Time) (bool, error) { shardDir := ShardDataDirPath(filePathPrefix, namespace, shard) checkpointPath := filesetPathFromTime(shardDir, blockStart, checkpointFileSuffix) - return FileExists(checkpointPath) + return CompleteCheckpointFileExists(checkpointPath) } // SnapshotFileSetExistsAt determines whether snapshot fileset files exist for the given namespace, shard, and block start time. @@ -1126,6 +1128,20 @@ func NextIndexSnapshotFileIndex(filePathPrefix string, namespace ident.ID, block return currentSnapshotIndex + 1, nil } +// CompleteCheckpointFileExists returns whether a checkpoint file exists, and if so, +// is it complete. +func CompleteCheckpointFileExists(filePath string) (bool, error) { + f, err := os.Stat(filePath) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + + return f.Size() == completeCheckpointFileSize, nil +} + // FileExists returns whether a file at the given path exists. func FileExists(filePath string) (bool, error) { _, err := os.Stat(filePath) From c06bc29db7a83ceeb9d8985dbb69c9641e2ff1a7 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 15 Oct 2018 14:21:54 -0400 Subject: [PATCH 02/11] fix tests --- src/dbnode/persist/fs/files.go | 4 +-- src/dbnode/persist/fs/files_test.go | 19 ++++++----- src/dbnode/persist/fs/persist_manager_test.go | 32 +++++++++++-------- src/dbnode/persist/fs/write.go | 4 +++ 4 files changed, 34 insertions(+), 25 deletions(-) diff --git a/src/dbnode/persist/fs/files.go b/src/dbnode/persist/fs/files.go index 31b4c4dced..0759ce8ee4 100644 --- a/src/dbnode/persist/fs/files.go +++ b/src/dbnode/persist/fs/files.go @@ -52,8 +52,6 @@ const ( commitLogComponentPosition = 2 indexFileSetComponentPosition = 2 - - completeCheckpointFileSize = 4 // bytes ) var ( @@ -1139,7 +1137,7 @@ func CompleteCheckpointFileExists(filePath string) (bool, error) { return false, err } - return f.Size() == completeCheckpointFileSize, nil + return f.Size() == checkpointFileSizeBytes, nil } // FileExists returns whether a file at the given path exists. diff --git a/src/dbnode/persist/fs/files_test.go b/src/dbnode/persist/fs/files_test.go index 15c9eaee90..eeefc9c2fb 100644 --- a/src/dbnode/persist/fs/files_test.go +++ b/src/dbnode/persist/fs/files_test.go @@ -282,24 +282,27 @@ func TestTimeAndVolumeIndexFromFileSetFilename(t *testing.T) { } func TestFileExists(t *testing.T) { - dir := createTempDir(t) - defer os.RemoveAll(dir) - shard := uint32(10) - start := time.Now() - shardDir := ShardDataDirPath(dir, testNs1ID, shard) - err := os.MkdirAll(shardDir, defaultNewDirectoryMode) + var ( + dir = createTempDir(t) + shard = uint32(10) + start = time.Now() + shardDir = ShardDataDirPath(dir, testNs1ID, shard) + checkpointFileBuf = make([]byte, checkpointFileSizeBytes) + err = os.MkdirAll(shardDir, defaultNewDirectoryMode) + ) + defer os.RemoveAll(dir) require.NoError(t, err) infoFilePath := filesetPathFromTime(shardDir, start, infoFileSuffix) - createDataFile(t, shardDir, start, infoFileSuffix, nil) + createDataFile(t, shardDir, start, infoFileSuffix, checkpointFileBuf) require.True(t, mustFileExists(t, infoFilePath)) exists, err := DataFileSetExistsAt(dir, testNs1ID, uint32(shard), start) require.NoError(t, err) require.False(t, exists) checkpointFilePath := filesetPathFromTime(shardDir, start, checkpointFileSuffix) - createDataFile(t, shardDir, start, checkpointFileSuffix, nil) + createDataFile(t, shardDir, start, checkpointFileSuffix, checkpointFileBuf) require.True(t, mustFileExists(t, checkpointFilePath)) exists, err = DataFileSetExistsAt(dir, testNs1ID, uint32(shard), start) require.NoError(t, err) diff --git a/src/dbnode/persist/fs/persist_manager_test.go b/src/dbnode/persist/fs/persist_manager_test.go index ae0b345203..0d8d2668b4 100644 --- a/src/dbnode/persist/fs/persist_manager_test.go +++ b/src/dbnode/persist/fs/persist_manager_test.go @@ -50,13 +50,14 @@ func TestPersistenceManagerPrepareDataFileExistsNoDelete(t *testing.T) { pm, _, _ := testDataPersistManager(t, ctrl) defer os.RemoveAll(pm.filePathPrefix) - shard := uint32(0) - blockStart := time.Unix(1000, 0) - shardDir := createDataShardDir(t, pm.filePathPrefix, testNs1ID, shard) - checkpointFilePath := filesetPathFromTime(shardDir, blockStart, checkpointFileSuffix) - f, err := os.Create(checkpointFilePath) - require.NoError(t, err) - f.Close() + var ( + shard = uint32(0) + blockStart = time.Unix(1000, 0) + shardDir = createDataShardDir(t, pm.filePathPrefix, testNs1ID, shard) + checkpointFilePath = filesetPathFromTime(shardDir, blockStart, checkpointFileSuffix) + checkpointFileBuf = make([]byte, checkpointFileSizeBytes) + ) + createFile(t, checkpointFilePath, checkpointFileBuf) flush, err := pm.StartDataPersist() require.NoError(t, err) @@ -83,8 +84,10 @@ func TestPersistenceManagerPrepareDataFileExistsWithDelete(t *testing.T) { pm, writer, _ := testDataPersistManager(t, ctrl) defer os.RemoveAll(pm.filePathPrefix) - shard := uint32(0) - blockStart := time.Unix(1000, 0) + var ( + shard = uint32(0) + blockStart = time.Unix(1000, 0) + ) writerOpts := xtest.CmpMatcher(DataWriterOpenOptions{ Identifier: FileSetFileIdentifier{ @@ -96,11 +99,12 @@ func TestPersistenceManagerPrepareDataFileExistsWithDelete(t *testing.T) { }, m3test.IdentTransformer) writer.EXPECT().Open(writerOpts).Return(nil) - shardDir := createDataShardDir(t, pm.filePathPrefix, testNs1ID, shard) - checkpointFilePath := filesetPathFromTime(shardDir, blockStart, checkpointFileSuffix) - f, err := os.Create(checkpointFilePath) - require.NoError(t, err) - f.Close() + var ( + shardDir = createDataShardDir(t, pm.filePathPrefix, testNs1ID, shard) + checkpointFilePath = filesetPathFromTime(shardDir, blockStart, checkpointFileSuffix) + checkpointFileBuf = make([]byte, checkpointFileSizeBytes) + ) + createFile(t, checkpointFilePath, checkpointFileBuf) flush, err := pm.StartDataPersist() require.NoError(t, err) diff --git a/src/dbnode/persist/fs/write.go b/src/dbnode/persist/fs/write.go index 90eb5e7916..e40b731ee3 100644 --- a/src/dbnode/persist/fs/write.go +++ b/src/dbnode/persist/fs/write.go @@ -40,6 +40,10 @@ import ( xtime "github.com/m3db/m3x/time" ) +const ( + checkpointFileSizeBytes = 4 +) + var ( errWriterEncodeTagsDataNotAccessible = errors.New( "failed to encode tags: cannot get data") From bad979a6da7483e77d1cc60a5332d61081d18ee4 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 15 Oct 2018 14:24:18 -0400 Subject: [PATCH 03/11] Replace use of FileExists with CompleteCheckpointFileExists --- src/dbnode/persist/fs/index_read.go | 2 +- src/dbnode/persist/fs/read.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dbnode/persist/fs/index_read.go b/src/dbnode/persist/fs/index_read.go index f36f03f2ed..3ed9a23f18 100644 --- a/src/dbnode/persist/fs/index_read.go +++ b/src/dbnode/persist/fs/index_read.go @@ -138,7 +138,7 @@ func (r *indexReader) Open( } func (r *indexReader) readCheckpointFile(filePath string) error { - exists, err := FileExists(filePath) + exists, err := CompleteCheckpointFileExists(filePath) if err != nil { return err } diff --git a/src/dbnode/persist/fs/read.go b/src/dbnode/persist/fs/read.go index 915efd8519..ebadde9647 100644 --- a/src/dbnode/persist/fs/read.go +++ b/src/dbnode/persist/fs/read.go @@ -248,7 +248,7 @@ func (r *reader) Status() DataFileSetReaderStatus { } func (r *reader) readCheckpointFile(filePath string) error { - exists, err := FileExists(filePath) + exists, err := CompleteCheckpointFileExists(filePath) if err != nil { return err } From d244413544da6fc3880d086a942e13b203e55c84 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 15 Oct 2018 14:30:01 -0400 Subject: [PATCH 04/11] Update tests and make FileExists return error for checkpoint files --- src/dbnode/persist/fs/files.go | 5 +++++ src/dbnode/persist/fs/files_test.go | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/dbnode/persist/fs/files.go b/src/dbnode/persist/fs/files.go index 0759ce8ee4..8932a9cff9 100644 --- a/src/dbnode/persist/fs/files.go +++ b/src/dbnode/persist/fs/files.go @@ -1142,6 +1142,11 @@ func CompleteCheckpointFileExists(filePath string) (bool, error) { // FileExists returns whether a file at the given path exists. func FileExists(filePath string) (bool, error) { + if strings.Contains(filePath, checkpointFileSuffix) { + return false, fmt.Errorf( + "tried to use FileExists to verify existence of checkpoint file: %s", filePath) + } + _, err := os.Stat(filePath) if err != nil { if os.IsNotExist(err) { diff --git a/src/dbnode/persist/fs/files_test.go b/src/dbnode/persist/fs/files_test.go index eeefc9c2fb..8bbd4a014d 100644 --- a/src/dbnode/persist/fs/files_test.go +++ b/src/dbnode/persist/fs/files_test.go @@ -303,11 +303,17 @@ func TestFileExists(t *testing.T) { checkpointFilePath := filesetPathFromTime(shardDir, start, checkpointFileSuffix) createDataFile(t, shardDir, start, checkpointFileSuffix, checkpointFileBuf) - require.True(t, mustFileExists(t, checkpointFilePath)) exists, err = DataFileSetExistsAt(dir, testNs1ID, uint32(shard), start) require.NoError(t, err) require.True(t, exists) + exists, err = CompleteCheckpointFileExists(checkpointFilePath) + require.NoError(t, err) + require.True(t, exists) + + _, err = FileExists(checkpointFilePath) + require.Error(t, err) + os.Remove(infoFilePath) require.False(t, mustFileExists(t, infoFilePath)) } From 8af3969083a17ac23fc7453732f67d2bdd8bdc61 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 15 Oct 2018 14:32:00 -0400 Subject: [PATCH 05/11] Fix test --- src/dbnode/persist/fs/read_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/dbnode/persist/fs/read_test.go b/src/dbnode/persist/fs/read_test.go index bf9637e5e9..1e9e38a7c1 100644 --- a/src/dbnode/persist/fs/read_test.go +++ b/src/dbnode/persist/fs/read_test.go @@ -269,9 +269,13 @@ func TestReadNoCheckpointFile(t *testing.T) { assert.NoError(t, err) assert.NoError(t, w.Close()) - shardDir := ShardDataDirPath(filePathPrefix, testNs1ID, shard) - checkpointFile := filesetPathFromTime(shardDir, testWriterStart, checkpointFileSuffix) - require.True(t, mustFileExists(t, checkpointFile)) + var ( + shardDir = ShardDataDirPath(filePathPrefix, testNs1ID, shard) + checkpointFile = filesetPathFromTime(shardDir, testWriterStart, checkpointFileSuffix) + ) + exists, err := CompleteCheckpointFileExists(checkpointFile) + require.NoError(t, err) + require.True(t, exists) os.Remove(checkpointFile) r := newTestReader(t, filePathPrefix) From 58ab6ecc22396dd465cce2cf37904c708899fdfe Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 15 Oct 2018 14:39:01 -0400 Subject: [PATCH 06/11] Update error message and add coment --- src/dbnode/persist/fs/files.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/dbnode/persist/fs/files.go b/src/dbnode/persist/fs/files.go index 8932a9cff9..233b43e074 100644 --- a/src/dbnode/persist/fs/files.go +++ b/src/dbnode/persist/fs/files.go @@ -40,6 +40,7 @@ import ( xclose "github.com/m3db/m3x/close" xerrors "github.com/m3db/m3x/errors" "github.com/m3db/m3x/ident" + "github.com/m3db/m3x/instrument" ) var timeZero time.Time @@ -1143,8 +1144,12 @@ func CompleteCheckpointFileExists(filePath string) (bool, error) { // FileExists returns whether a file at the given path exists. func FileExists(filePath string) (bool, error) { if strings.Contains(filePath, checkpointFileSuffix) { + // Existence of a checkpoint file needs to be verified using the function + // CompleteCheckpointFileExists instead to ensure that it has been + // completely written out. return false, fmt.Errorf( - "tried to use FileExists to verify existence of checkpoint file: %s", filePath) + "%s tried to use FileExists to verify existence of checkpoint file: %s", + instrument.InvariantViolatedMetricName, filePath) } _, err := os.Stat(filePath) From 4094224471262e1628ce2090e3857d65fc22024d Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 15 Oct 2018 15:26:06 -0400 Subject: [PATCH 07/11] Add additional test for CompleteCheckpointFileExists --- src/dbnode/persist/fs/files_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/dbnode/persist/fs/files_test.go b/src/dbnode/persist/fs/files_test.go index 8bbd4a014d..c4356c2d7e 100644 --- a/src/dbnode/persist/fs/files_test.go +++ b/src/dbnode/persist/fs/files_test.go @@ -318,6 +318,32 @@ func TestFileExists(t *testing.T) { require.False(t, mustFileExists(t, infoFilePath)) } +func TestCompleteCheckpointFileExists(t *testing.T) { + var ( + dir = createTempDir(t) + shard = uint32(10) + start = time.Now() + shardDir = ShardDataDirPath(dir, testNs1ID, shard) + checkpointFilePath = filesetPathFromTime(shardDir, start, checkpointFileSuffix) + err = os.MkdirAll(shardDir, defaultNewDirectoryMode) + + validCheckpointFileBuf = make([]byte, checkpointFileSizeBytes) + invalidCheckpointFileBuf = make([]byte, checkpointFileSizeBytes+1) + ) + defer os.RemoveAll(dir) + require.NoError(t, err) + + createDataFile(t, shardDir, start, checkpointFileSuffix, invalidCheckpointFileBuf) + exists, err := CompleteCheckpointFileExists(checkpointFilePath) + require.NoError(t, err) + require.False(t, exists) + + createDataFile(t, shardDir, start, checkpointFileSuffix, validCheckpointFileBuf) + exists, err = CompleteCheckpointFileExists(checkpointFilePath) + require.NoError(t, err) + require.True(t, exists) +} + func TestShardDirPath(t *testing.T) { require.Equal(t, "foo/bar/data/testNs/12", ShardDataDirPath("foo/bar", testNs1ID, 12)) require.Equal(t, "foo/bar/data/testNs/12", ShardDataDirPath("foo/bar/", testNs1ID, 12)) From d1048d80fe8f1e3506f8d8bb424953c51ef4f26f Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 15 Oct 2018 15:26:35 -0400 Subject: [PATCH 08/11] Add comment --- src/dbnode/persist/fs/files.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/dbnode/persist/fs/files.go b/src/dbnode/persist/fs/files.go index 233b43e074..99fc837413 100644 --- a/src/dbnode/persist/fs/files.go +++ b/src/dbnode/persist/fs/files.go @@ -1138,6 +1138,8 @@ func CompleteCheckpointFileExists(filePath string) (bool, error) { return false, err } + // Make sure the checkpoint file was completely written out and its + // not just an empty file. return f.Size() == checkpointFileSizeBytes, nil } From 86fa49bf16b8f3d5bcad87db2d590ba04ccff8f3 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Mon, 15 Oct 2018 15:31:52 -0400 Subject: [PATCH 09/11] Refactor constants and add test --- src/dbnode/digest/buffer.go | 8 ++++---- src/dbnode/persist/fs/files.go | 2 +- src/dbnode/persist/fs/files_test.go | 6 +++--- src/dbnode/persist/fs/persist_manager_test.go | 4 ++-- src/dbnode/persist/fs/read_write_test.go | 6 ++++++ src/dbnode/persist/fs/write.go | 3 ++- 6 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/dbnode/digest/buffer.go b/src/dbnode/digest/buffer.go index 3e427a3f90..a5cbeb8c77 100644 --- a/src/dbnode/digest/buffer.go +++ b/src/dbnode/digest/buffer.go @@ -26,8 +26,8 @@ import ( ) const ( - // digest size is 4 bytes - digestLen = 4 + // DigestLenBytes is the length of generated digests in bytes. + DigestLenBytes = 4 ) var ( @@ -40,7 +40,7 @@ type Buffer []byte // NewBuffer creates a new digest buffer. func NewBuffer() Buffer { - return make([]byte, digestLen) + return make([]byte, DigestLenBytes) } // WriteDigest writes a digest to the buffer. @@ -71,5 +71,5 @@ func (b Buffer) ReadDigestFromFile(fd *os.File) (uint32, error) { // ToBuffer converts a byte slice to a digest buffer. func ToBuffer(buf []byte) Buffer { - return Buffer(buf[:digestLen]) + return Buffer(buf[:DigestLenBytes]) } diff --git a/src/dbnode/persist/fs/files.go b/src/dbnode/persist/fs/files.go index 99fc837413..0f49a09223 100644 --- a/src/dbnode/persist/fs/files.go +++ b/src/dbnode/persist/fs/files.go @@ -1140,7 +1140,7 @@ func CompleteCheckpointFileExists(filePath string) (bool, error) { // Make sure the checkpoint file was completely written out and its // not just an empty file. - return f.Size() == checkpointFileSizeBytes, nil + return f.Size() == CheckpointFileSizeBytes, nil } // FileExists returns whether a file at the given path exists. diff --git a/src/dbnode/persist/fs/files_test.go b/src/dbnode/persist/fs/files_test.go index c4356c2d7e..fb94adac1a 100644 --- a/src/dbnode/persist/fs/files_test.go +++ b/src/dbnode/persist/fs/files_test.go @@ -288,7 +288,7 @@ func TestFileExists(t *testing.T) { shard = uint32(10) start = time.Now() shardDir = ShardDataDirPath(dir, testNs1ID, shard) - checkpointFileBuf = make([]byte, checkpointFileSizeBytes) + checkpointFileBuf = make([]byte, CheckpointFileSizeBytes) err = os.MkdirAll(shardDir, defaultNewDirectoryMode) ) defer os.RemoveAll(dir) @@ -327,8 +327,8 @@ func TestCompleteCheckpointFileExists(t *testing.T) { checkpointFilePath = filesetPathFromTime(shardDir, start, checkpointFileSuffix) err = os.MkdirAll(shardDir, defaultNewDirectoryMode) - validCheckpointFileBuf = make([]byte, checkpointFileSizeBytes) - invalidCheckpointFileBuf = make([]byte, checkpointFileSizeBytes+1) + validCheckpointFileBuf = make([]byte, CheckpointFileSizeBytes) + invalidCheckpointFileBuf = make([]byte, CheckpointFileSizeBytes+1) ) defer os.RemoveAll(dir) require.NoError(t, err) diff --git a/src/dbnode/persist/fs/persist_manager_test.go b/src/dbnode/persist/fs/persist_manager_test.go index 0d8d2668b4..a41e3a6eab 100644 --- a/src/dbnode/persist/fs/persist_manager_test.go +++ b/src/dbnode/persist/fs/persist_manager_test.go @@ -55,7 +55,7 @@ func TestPersistenceManagerPrepareDataFileExistsNoDelete(t *testing.T) { blockStart = time.Unix(1000, 0) shardDir = createDataShardDir(t, pm.filePathPrefix, testNs1ID, shard) checkpointFilePath = filesetPathFromTime(shardDir, blockStart, checkpointFileSuffix) - checkpointFileBuf = make([]byte, checkpointFileSizeBytes) + checkpointFileBuf = make([]byte, CheckpointFileSizeBytes) ) createFile(t, checkpointFilePath, checkpointFileBuf) @@ -102,7 +102,7 @@ func TestPersistenceManagerPrepareDataFileExistsWithDelete(t *testing.T) { var ( shardDir = createDataShardDir(t, pm.filePathPrefix, testNs1ID, shard) checkpointFilePath = filesetPathFromTime(shardDir, blockStart, checkpointFileSuffix) - checkpointFileBuf = make([]byte, checkpointFileSizeBytes) + checkpointFileBuf = make([]byte, CheckpointFileSizeBytes) ) createFile(t, checkpointFilePath, checkpointFileBuf) diff --git a/src/dbnode/persist/fs/read_write_test.go b/src/dbnode/persist/fs/read_write_test.go index 7435a6e909..1bed65effd 100644 --- a/src/dbnode/persist/fs/read_write_test.go +++ b/src/dbnode/persist/fs/read_write_test.go @@ -253,6 +253,12 @@ func TestSimpleReadWrite(t *testing.T) { readTestData(t, r, 0, testWriterStart, entries) } +func TestCheckpointFileSizeBytesSize(t *testing.T) { + // These values need to match so that the logic for determining whether + // a checkpoint file is complete or not remains correct. + require.Equal(t, digest.DigestLenBytes, CheckpointFileSizeBytes) +} + func TestDuplicateWrite(t *testing.T) { dir := createTempDir(t) filePathPrefix := filepath.Join(dir, "") diff --git a/src/dbnode/persist/fs/write.go b/src/dbnode/persist/fs/write.go index e40b731ee3..8b4ba3a2ed 100644 --- a/src/dbnode/persist/fs/write.go +++ b/src/dbnode/persist/fs/write.go @@ -41,7 +41,8 @@ import ( ) const ( - checkpointFileSizeBytes = 4 + // CheckpointFileSizeBytes is the expected size of a valid checkpoint file. + CheckpointFileSizeBytes = 4 ) var ( From 0f82f3a571cfe3ff109859f327c49b27d645b5e2 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Tue, 16 Oct 2018 10:11:40 -0400 Subject: [PATCH 10/11] Make sure file is checkpoint in completecheckpointfileexists --- src/dbnode/persist/fs/files.go | 6 ++++++ src/dbnode/persist/fs/files_test.go | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/src/dbnode/persist/fs/files.go b/src/dbnode/persist/fs/files.go index 0f49a09223..50e890a3d0 100644 --- a/src/dbnode/persist/fs/files.go +++ b/src/dbnode/persist/fs/files.go @@ -1130,6 +1130,12 @@ func NextIndexSnapshotFileIndex(filePathPrefix string, namespace ident.ID, block // CompleteCheckpointFileExists returns whether a checkpoint file exists, and if so, // is it complete. func CompleteCheckpointFileExists(filePath string) (bool, error) { + if !strings.Contains(filePath, checkpointFileSuffix) { + return false, fmt.Errorf( + "%s tried to use CompleteCheckpointFileExists to verify existence of checkpoint file: %s", + instrument.InvariantViolatedMetricName, filePath) + } + f, err := os.Stat(filePath) if err != nil { if os.IsNotExist(err) { diff --git a/src/dbnode/persist/fs/files_test.go b/src/dbnode/persist/fs/files_test.go index fb94adac1a..b4ef411bfa 100644 --- a/src/dbnode/persist/fs/files_test.go +++ b/src/dbnode/persist/fs/files_test.go @@ -38,6 +38,7 @@ import ( "github.com/m3db/m3/src/dbnode/retention" "github.com/m3db/m3/src/dbnode/storage/namespace" "github.com/m3db/m3x/ident" + "github.com/m3db/m3x/instrument" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -342,6 +343,10 @@ func TestCompleteCheckpointFileExists(t *testing.T) { exists, err = CompleteCheckpointFileExists(checkpointFilePath) require.NoError(t, err) require.True(t, exists) + + exists, err = CompleteCheckpointFileExists("some-arbitrary-file") + require.Contains(t, err.Error(), instrument.InvariantViolatedMetricName) + require.False(t, exists) } func TestShardDirPath(t *testing.T) { From b47f7b0af61e1b88e82089897b0cbd255b1c4ee1 Mon Sep 17 00:00:00 2001 From: Richard Artoul Date: Tue, 16 Oct 2018 10:30:28 -0400 Subject: [PATCH 11/11] Fix error message --- src/dbnode/persist/fs/files.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dbnode/persist/fs/files.go b/src/dbnode/persist/fs/files.go index 50e890a3d0..fa110708e9 100644 --- a/src/dbnode/persist/fs/files.go +++ b/src/dbnode/persist/fs/files.go @@ -1132,7 +1132,7 @@ func NextIndexSnapshotFileIndex(filePathPrefix string, namespace ident.ID, block func CompleteCheckpointFileExists(filePath string) (bool, error) { if !strings.Contains(filePath, checkpointFileSuffix) { return false, fmt.Errorf( - "%s tried to use CompleteCheckpointFileExists to verify existence of checkpoint file: %s", + "%s tried to use CompleteCheckpointFileExists to verify existence of non checkpoint file: %s", instrument.InvariantViolatedMetricName, filePath) }