Skip to content

Commit c2f6e81

Browse files
leehinmanmergify[bot]
authored andcommitted
make statestore creation of meta.json atomic (#51897)
* make statestore creation of meta.json atomic * add changelog * fix linter issues * address scenario that Anderson caught * fix linter issue (cherry picked from commit 052272d)
1 parent c9fd1ae commit c2f6e81

4 files changed

Lines changed: 104 additions & 20 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
kind: bug-fix
2+
3+
summary: Prevent statestore startup failure when meta.json is left empty after an unclean shutdown
4+
5+
component: libbeat
6+
7+
pr: https://github.com/elastic/beats/pull/51897

libbeat/statestore/backend/memlog/diskstore.go

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"strconv"
3030

3131
"github.com/elastic/beats/v7/libbeat/common/cleanup"
32+
agentfile "github.com/elastic/elastic-agent-libs/file"
3233
"github.com/elastic/elastic-agent-libs/logp"
3334
"github.com/elastic/elastic-agent-libs/mapstr"
3435
)
@@ -195,7 +196,7 @@ func (s *diskstore) tryOpenLog() error {
195196
return err
196197
}
197198

198-
s.logFileSize = uint64(info.Size())
199+
s.logFileSize = uint64(info.Size()) //nolint:gosec // G115: info.Size() is non-negative for a regular file
199200
}
200201

201202
ok = true
@@ -640,31 +641,35 @@ func checkMeta(meta storeMeta) error {
640641

641642
func writeMetaFile(home string, mode os.FileMode) error {
642643
path := filepath.Join(home, metaFileName)
643-
f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR|os.O_TRUNC, mode)
644+
f, err := os.CreateTemp(home, metaFileName+".tmp-*")
644645
if err != nil {
645-
return err
646+
return fmt.Errorf("error creating tmp meta file: %w", err)
646647
}
648+
tmpName := f.Name()
649+
defer func() {
650+
_ = f.Close()
651+
_ = os.Remove(tmpName)
652+
}()
647653

648-
ok := false
649-
defer cleanup.IfNot(&ok, func() {
650-
f.Close()
651-
})
654+
if err = f.Chmod(mode); err != nil {
655+
return fmt.Errorf("error setting tmp meta file permissions: %w", err)
656+
}
652657

653658
enc := newJSONEncoder(f)
654-
err = enc.Encode(storeMeta{
655-
Version: storeVersion,
656-
})
657-
if err != nil {
658-
return err
659+
if err = enc.Encode(storeMeta{Version: storeVersion}); err != nil {
660+
return fmt.Errorf("error encoding meta file: %w", err)
659661
}
660662

661-
if err := f.Sync(); err != nil {
662-
return err
663+
if err = f.Sync(); err != nil {
664+
return fmt.Errorf("error syncing meta file: %w", err)
663665
}
664666

665-
ok = true
666-
if err := f.Close(); err != nil {
667-
return err
667+
if err = f.Close(); err != nil {
668+
return fmt.Errorf("error closing tmp meta file: %w", err)
669+
}
670+
671+
if err = agentfile.SafeFileRotate(path, tmpName); err != nil {
672+
return fmt.Errorf("error rotating meta file into place: %w", err)
668673
}
669674

670675
trySyncPath(home)
@@ -695,5 +700,5 @@ func readMetaFile(home string) (storeMeta, error) {
695700
// - a < b (mod 2^63)
696701
// - b > a after an integer rollover that is still within the distance of <2^63-1
697702
func isTxIDLessEqual(a, b uint64) bool {
698-
return int64(a-b) <= 0
703+
return a == b || a-b >= 1<<63
699704
}

libbeat/statestore/backend/memlog/store.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ type entry struct {
6363
func openStore(log *logp.Logger, home string, mode os.FileMode, bufSz uint, ignoreVersionCheck bool, checkpoint CheckpointPredicate) (*store, error) {
6464
fi, err := os.Stat(home)
6565
if os.IsNotExist(err) {
66-
err = os.MkdirAll(home, os.ModeDir|0770)
66+
err = os.MkdirAll(home, os.ModeDir|0o770)
6767
if err != nil {
6868
return nil, err
6969
}
@@ -75,7 +75,16 @@ func openStore(log *logp.Logger, home string, mode os.FileMode, bufSz uint, igno
7575
} else if !fi.Mode().IsDir() {
7676
return nil, fmt.Errorf("'%v' is not a directory", home)
7777
} else {
78-
if err := pathEnsurePermissions(filepath.Join(home, metaFileName), mode); err != nil {
78+
metaPath := filepath.Join(home, metaFileName)
79+
if _, err := os.Stat(metaPath); os.IsNotExist(err) {
80+
// Directory exists but meta.json is absent — the process likely crashed
81+
// after MkdirAll but before writeMetaFile completed on a prior run.
82+
if err := writeMetaFile(home, mode); err != nil {
83+
return nil, err
84+
}
85+
} else if err != nil {
86+
return nil, fmt.Errorf("failed to stat meta file: %w", err)
87+
} else if err := pathEnsurePermissions(metaPath, mode); err != nil {
7988
return nil, fmt.Errorf("failed to update meta file permissions: %w", err)
8089
}
8190
}

libbeat/statestore/backend/memlog/store_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,75 @@ import (
2222
"path/filepath"
2323
"testing"
2424

25+
"github.com/stretchr/testify/assert"
2526
"github.com/stretchr/testify/require"
2627

2728
"github.com/elastic/elastic-agent-libs/logp/logptest"
2829
"github.com/elastic/elastic-agent-libs/mapstr"
2930
)
3031

32+
func TestWriteMetaFileAtomic(t *testing.T) {
33+
t.Run("writes_valid_meta_and_cleans_up_temp", func(t *testing.T) {
34+
dir := t.TempDir()
35+
36+
require.NoError(t, writeMetaFile(dir, 0o600))
37+
38+
meta, err := readMetaFile(dir)
39+
require.NoError(t, err)
40+
require.NoError(t, checkMeta(meta))
41+
42+
matches, err := filepath.Glob(filepath.Join(dir, "*.tmp-*"))
43+
require.NoError(t, err)
44+
assert.Empty(t, matches, "no temp files should remain after successful write")
45+
})
46+
47+
t.Run("no_partial_file_on_rotate_failure", func(t *testing.T) {
48+
dir := t.TempDir()
49+
// Place a directory at the meta.json path so SafeFileRotate (rename) fails.
50+
// This simulates a crash-safe scenario: the destination is never touched
51+
// until the temp file is fully written and synced.
52+
metaPath := filepath.Join(dir, metaFileName)
53+
require.NoError(t, os.Mkdir(metaPath, 0o700))
54+
55+
err := writeMetaFile(dir, 0o600)
56+
require.Error(t, err)
57+
58+
// The directory at the destination must be intact (not replaced or emptied).
59+
fi, statErr := os.Stat(metaPath)
60+
require.NoError(t, statErr)
61+
assert.True(t, fi.IsDir(), "destination should still be a directory after failed rotate")
62+
63+
// No temp files should remain.
64+
matches, globErr := filepath.Glob(filepath.Join(dir, "*.tmp-*"))
65+
require.NoError(t, globErr)
66+
assert.Empty(t, matches, "no temp files should remain after failed rotate")
67+
})
68+
}
69+
70+
// TestOpenStoreRecoversMissingMeta verifies that openStore creates meta.json when
71+
// the store directory already exists but meta.json is absent — the scenario that
72+
// arises when the process crashes after MkdirAll but before writeMetaFile on a
73+
// prior run.
74+
func TestOpenStoreRecoversMissingMeta(t *testing.T) {
75+
dir := t.TempDir()
76+
// Simulate a crash after MkdirAll: the directory exists but is empty.
77+
storePath := filepath.Join(dir, "store")
78+
require.NoError(t, os.Mkdir(storePath, 0o770))
79+
80+
logger := logptest.NewTestingLogger(t, "")
81+
store, err := openStore(logger.Named("test"), storePath, 0o660, 4096, false, func(_ uint64) bool {
82+
return false
83+
})
84+
require.NoError(t, err, "openStore must succeed even when meta.json was missing")
85+
store.Close()
86+
87+
require.FileExists(t, filepath.Join(storePath, metaFileName), "meta.json must be created on recovery")
88+
89+
meta, err := readMetaFile(storePath)
90+
require.NoError(t, err)
91+
assert.NoError(t, checkMeta(meta))
92+
}
93+
3194
func TestRecoverFromCorruption(t *testing.T) {
3295
path := t.TempDir()
3396

0 commit comments

Comments
 (0)