Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Merged by Bors] - support existing multiple bootstrap files #4875

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - support existing multiple bootstrap files #4875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 33 additions & 2 deletions bootstrap/updater.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"os"
"path/filepath"
"strconv"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -212,7 +213,7 @@
switch suffix {
case SuffixActiveSet, SuffixBeacon, SuffixBoostrap:
default:
u.logger.With().Fatal("unexpected suffix for fallback files", log.String("suffix", suffix))
return

Check warning on line 216 in bootstrap/updater.go

View check run for this annotation

Codecov / codecov/patch

bootstrap/updater.go#L216 

Added line #L216 was not covered by tests
}
if _, ok := u.updates[epoch]; !ok {
u.updates[epoch] = map[string]struct{}{}
Expand Down Expand Up @@ -419,6 +420,33 @@
return verified, nil
}

func renameLegacyFile(fs afero.Fs, path string) string {
var idx int
for _, suffix := range []string{SuffixBoostrap, SuffixBeacon, SuffixActiveSet} {
if strings.HasSuffix(path, suffix) {
return path
}
}
for _, suffix := range []string{SuffixBoostrap, SuffixBeacon, SuffixActiveSet} {
idx = strings.Index(path, fmt.Sprintf("-%s-", suffix))
if idx > -1 {
break
}
}
if idx < 0 {
return ""
}
newPath := path[:idx+suffixLen+1]
if exists, _ := afero.Exists(fs, newPath); exists {
_ = fs.Remove(path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this cause issues? Removing an existing file to make space for the old file to be renamed with new name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you start a node repeatedly, you will see the following.

$ ls -l data/bootstrap/2
total 1404
-r-------- 1 k k 158713 Aug 15 22:12 epoch-2-update-bs-2023-08-15T20-12-53
-r-------- 1 k k 158713 Aug 15 22:53 epoch-2-update-bs-2023-08-15T20-53-20
-r-------- 1 k k 158713 Aug 15 23:23 epoch-2-update-bs-2023-08-15T21-23-44
-r-------- 1 k k 158713 Aug 16 02:39 epoch-2-update-bs-2023-08-16T00-39-21
-r-------- 1 k k 158713 Aug 16 02:47 epoch-2-update-bs-2023-08-16T00-47-43
-r-------- 1 k k 158713 Aug 16 03:01 epoch-2-update-bs-2023-08-16T01-01-58
-r-------- 1 k k 158713 Aug 16 19:26 epoch-2-update-bs-2023-08-16T17-26-07
-r-------- 1 k k 158713 Aug 16 20:15 epoch-2-update-bs-2023-08-16T18-15-46
-r-------- 1 k k 158713 Aug 16 20:16 epoch-2-update-bs-2023-08-16T18-16-00

those files are all identical. we just need one, and newPath already exists (because it renamed the first one to newPath on line 444. so it's safe to delete here.

return ""
}
if err := fs.Rename(path, newPath); err != nil {
return ""
}

Check warning on line 446 in bootstrap/updater.go

View check run for this annotation

Codecov / codecov/patch

bootstrap/updater.go#L445-L446 

Added lines #L445 - L446 were not covered by tests
return newPath
}

func load(fs afero.Fs, cfg Config, current types.EpochID) ([]*VerifiedUpdate, error) {
dir := bootstrapDir(cfg.DataDir)
_, err := fs.Stat(dir)
Expand All @@ -437,7 +465,10 @@
return nil, fmt.Errorf("read epoch dir %v: %w", dir, err)
}
for _, f := range files {
persisted := filepath.Join(edir, f.Name())
persisted := renameLegacyFile(fs, filepath.Join(edir, f.Name()))
if persisted == "" {
continue
}
data, err := afero.ReadFile(fs, persisted)
if err != nil {
return nil, fmt.Errorf("read bootstrap file %v: %w", persisted, err)
Expand Down
95 changes: 93 additions & 2 deletions bootstrap/updater_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,97 @@ func checkUpdate4(t *testing.T, got *bootstrap.VerifiedUpdate) {

type checkFunc func(*testing.T, *bootstrap.VerifiedUpdate)

func TestLoad_BackwardCompatible(t *testing.T) {
epoch := 2
tcs := []struct {
desc string
filenames []string
persisted, suffix string
}{
{
desc: "legacy timestamp bootstrap",
filenames: []string{"epoch-2-update-bs-2023-08-10T06-00-08", "epoch-2-update-bs-2023-08-10T08-00-09"},
persisted: "epoch-2-update-bs",
suffix: bootstrap.SuffixBoostrap,
},
{
desc: "legacy timestamp beacon",
filenames: []string{"epoch-2-update-bc-2023-08-10T06-00-08", "epoch-2-update-bc-2023-08-10T08-00-09"},
persisted: "epoch-2-update-bc",
suffix: bootstrap.SuffixBeacon,
},
{
desc: "legacy timestamp active set",
filenames: []string{"epoch-2-update-as-2023-08-10T06-00-08", "epoch-2-update-as-2023-08-10T08-00-09"},
persisted: "epoch-2-update-as",
suffix: bootstrap.SuffixActiveSet,
},
{
desc: "legacy timestamp huh",
filenames: []string{"epoch-2-update-huh-2023-08-10T06-00-08"},
},
{
desc: "bootstrap",
filenames: []string{"epoch-2-update-bs"},
persisted: "epoch-2-update-bs",
suffix: bootstrap.SuffixBoostrap,
},
{
desc: "beacon",
filenames: []string{"epoch-2-update-bc"},
persisted: "epoch-2-update-bc",
suffix: bootstrap.SuffixBeacon,
},
{
desc: "active set",
filenames: []string{"epoch-2-update-as"},
persisted: "epoch-2-update-as",
suffix: bootstrap.SuffixActiveSet,
},
}
for _, tc := range tcs {
tc := tc
t.Run(tc.desc, func(t *testing.T) {
t.Parallel()

cfg := bootstrap.DefaultConfig()
fs := afero.NewMemMapFs()
persistDir := filepath.Join(cfg.DataDir, bootstrap.DirName)
for _, file := range tc.filenames {
path := filepath.Join(persistDir, strconv.Itoa(epoch), file)
require.NoError(t, fs.MkdirAll(path, 0o700))
require.NoError(t, afero.WriteFile(fs, path, []byte(update2), 0o400))
}
mc := bootstrap.NewMocklayerClock(gomock.NewController(t))
mc.EXPECT().CurrentLayer().Return(current.FirstLayer())
updater := bootstrap.New(
mc,
bootstrap.WithConfig(cfg),
bootstrap.WithLogger(logtest.New(t)),
bootstrap.WithFilesystem(fs),
)
require.NoError(t, updater.Load(context.Background()))
if tc.suffix != "" {
require.True(t, updater.Downloaded(types.EpochID(epoch), tc.suffix))
} else {
require.False(t, updater.Downloaded(types.EpochID(epoch), bootstrap.SuffixBoostrap))
require.False(t, updater.Downloaded(types.EpochID(epoch), bootstrap.SuffixActiveSet))
require.False(t, updater.Downloaded(types.EpochID(epoch), bootstrap.SuffixBeacon))
}
if tc.persisted != "" {
edir := filepath.Join(persistDir, strconv.Itoa(epoch))
exists, err := afero.Exists(fs, filepath.Join(edir, tc.persisted))
require.NoError(t, err)
require.True(t, exists)
all, err := afero.ReadDir(fs, edir)
require.NoError(t, err)
require.Len(t, all, 1)
require.Equal(t, tc.persisted, all[0].Name())
}
})
}
}

func TestLoad(t *testing.T) {
tcs := []struct {
desc string
Expand All @@ -136,12 +227,12 @@ func TestLoad(t *testing.T) {
desc: "recovery required",
persisted: map[types.EpochID][]string{
current - 2: {bootstrap.SuffixBoostrap, update1},
current - 1: {bootstrap.SuffixBoostrap, update2},
current - 1: {bootstrap.SuffixActiveSet, update2},
current: {bootstrap.SuffixBeacon, update3},
current + 1: {bootstrap.SuffixActiveSet, update4},
},
cached: map[types.EpochID]string{
current - 1: bootstrap.SuffixBoostrap,
current - 1: bootstrap.SuffixActiveSet,
current: bootstrap.SuffixBeacon,
current + 1: bootstrap.SuffixActiveSet,
},
Expand Down