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

Add option to keep redundant files #263

Closed
wants to merge 1 commit into from
Closed

Add option to keep redundant files #263

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 33 additions & 17 deletions initialization/initialization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ type option struct {
logger *Logger
powDifficultyFunc func(uint64) []byte
referenceOracle *oracle.WorkOracle

keepRedundantFiles bool
}

func (o *option) validate() error {
Expand Down Expand Up @@ -135,6 +137,13 @@ func WithLogger(logger *zap.Logger) OptionFunc {
}
}

func WithKeepRedundantFiles() OptionFunc {
return func(opts *option) error {
opts.keepRedundantFiles = true
return nil
}
}

// withDifficultyFunc sets the difficulty function for the initializer.
// NOTE: This is an internal option for tests and should not be used by external packages.
func withDifficultyFunc(powDifficultyFunc func(uint64) []byte) OptionFunc {
Expand Down Expand Up @@ -182,6 +191,8 @@ type Initializer struct {
logger *Logger
referenceOracle *oracle.WorkOracle
powDifficultyFunc func(uint64) []byte

keepRedundantFiles bool
}

func NewInitializer(opts ...OptionFunc) (*Initializer, error) {
Expand All @@ -202,15 +213,16 @@ func NewInitializer(opts ...OptionFunc) (*Initializer, error) {
}

init := &Initializer{
cfg: *options.cfg,
opts: *options.initOpts,
nodeId: options.nodeId,
commitmentAtxId: options.commitmentAtxId,
commitment: options.commitment,
diskState: NewDiskState(options.initOpts.DataDir, uint(config.BitsPerLabel)),
logger: options.logger,
powDifficultyFunc: options.powDifficultyFunc,
referenceOracle: options.referenceOracle,
cfg: *options.cfg,
opts: *options.initOpts,
nodeId: options.nodeId,
commitmentAtxId: options.commitmentAtxId,
commitment: options.commitment,
diskState: NewDiskState(options.initOpts.DataDir, uint(config.BitsPerLabel)),
logger: options.logger,
powDifficultyFunc: options.powDifficultyFunc,
referenceOracle: options.referenceOracle,
keepRedundantFiles: options.keepRedundantFiles,
}

numLabelsWritten, err := init.diskState.NumLabelsWritten()
Expand Down Expand Up @@ -295,7 +307,7 @@ func (init *Initializer) Initialize(ctx context.Context) error {
zap.Int("firstFileIndex", layout.FirstFileIdx),
zap.Int("lastFileIndex", layout.LastFileIdx),
)
if err := removeRedundantFiles(init.cfg, init.opts, init.logger); err != nil {
if err := checkRedundantFiles(init.cfg, init.opts, init.keepRedundantFiles, init.logger); err != nil {
return err
}
Comment on lines -298 to 312
Copy link
Member

Choose a reason for hiding this comment

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

Instead of passing the bool to the function I'd prefer to instead not call removeRedundantFiles when it is true:

if !init.keepRedundantFiles {
	if err := removeRedundantFiles(init.cfg, init.opts, init.logger); err != nil {
		return err
	}
}

no need to iterate over all files telling the user what is all ignored when the user actively chose to ignore files.

Copy link
Author

@xearl4 xearl4 Jan 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Certainly fine as well.

But in the usage scenarios I envision, many users will set "keep" as a default everywhere, and then still find it helpful to see log messages where additional files are discovered, to assist them in keeping track of things. The log message helps in cases where one forgot to adjust the num-units after an increase; it also helps in cleaning up after a decrease: it reminds the user to think about a situation that was previously handled in a one-sided manner by the program by deleting files.

In fact, I originally had exactly the variant you proposed, with no log message at all :) I only added the log message because I thought that might make it easier to upstream. Having used it while resizing (mostly increasing) lots of postdata over the past few weeks, I have come to slightly prefer the log message.

All that said: either way is fine with me.


Expand Down Expand Up @@ -395,10 +407,10 @@ func (init *Initializer) Initialize(ctx context.Context) error {
return fmt.Errorf("no nonce found")
}

func removeRedundantFiles(cfg config.Config, opts config.InitOpts, logger *zap.Logger) error {
// Go over all postdata_N.bin files in the data directory and remove the ones that are not needed.
func checkRedundantFiles(cfg config.Config, opts config.InitOpts, keep bool, logger *zap.Logger) error {
// Go over all postdata_N.bin files in the data directory and check for files that are not needed.
// The files with indices from 0 to init.opts.TotalFiles(init.cfg.LabelsPerUnit) - 1 are preserved.
// The rest are redundant and can be removed.
// The rest are redundant.
maxFileIndex := opts.TotalFiles(cfg.LabelsPerUnit) - 1
logger.Debug("attempting to remove redundant files above index", zap.Int("maxFileIndex", maxFileIndex))

Expand All @@ -414,10 +426,14 @@ func removeRedundantFiles(cfg config.Config, opts config.InitOpts, logger *zap.L
continue
}
if fileIndex > maxFileIndex {
logger.Info("removing redundant file", zap.String("fileName", name))
path := filepath.Join(opts.DataDir, name)
if err := os.Remove(path); err != nil {
return fmt.Errorf("failed to delete file (%v): %w", path, err)
if keep {
logger.Info("ignoring redundant file", zap.String("fileName", name))
} else {
logger.Info("removing redundant file", zap.String("fileName", name))
path := filepath.Join(opts.DataDir, name)
if err := os.Remove(path); err != nil {
return fmt.Errorf("failed to delete file (%v): %w", path, err)
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion initialization/initialization_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1113,7 +1113,7 @@ func TestRemoveRedundantFiles(t *testing.T) {
require.NoError(t, f.Close())
}

removeRedundantFiles(cfg, opts, zap.NewNop())
checkRedundantFiles(cfg, opts, false, zap.NewNop())

files, err := os.ReadDir(opts.DataDir)
require.NoError(t, err)
Expand Down