Add option to keep redundant files #263
Conversation
Default behaviour is preserved: redundant files are deleted. If the new option is enabled, an info entry is logged for redundant files, but they are not deleted. This is in preparation for a forthcoming go-spacemesh change using this new option.
There was a problem hiding this comment.
Thanks for your contributions! 🙂
postcli in post/cmd/postcli might also benefit from the new functional option of the Initializer, if you find some time to take a look at it.
You might want to consider adding the KeepRedundantFiles flag to InitOpts in post/config/config.go instead of using a dedicated functional option for it. This will probably make integration in go-spacemesh easier.
Setting the flag via the cmd line and/or config is a bit involved - I suggest you add the bool to PostSetupOpts in go-spacemesh/activation/post.go which is already passed to the service that uses the Initializer to generate the post data - PostSetupManager in go-spacemesh/activation/post.go. PostSetupOpts also has a method to convert it to config.InitOpts used by the Initializer which you can extend with the new flag.
If you want to expose the flag not only via the node-config.json but also via cmdline parameters you can take a look at go-spacemesh/cmd/root.go to see how other cmd line parameters are handled. 🙂
| 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 | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Sure. Main question is: what do we want the default to be? Should |
|
On the other hand having a |
|
I agree with @fasmat here. by default, it should not delete UNLESS specified. postcli case is a bit specific because we support |
|
Hi @xearl4 @schinzelh, we just merged a change that adds additional checks to With the upcoming I will therefore close this PR. If you feel additional changes are needed feel free to re-open and update your PR and re-start the discussion 🙂 |
|
Sounds good, thanks! |
A long-standing pain point for go-spacemesh users is that it simply deletes postdata files that are deemed "redundant". For a go-spacemesh managed through smapp, the behaviour may be reasonable and comprehensible. For CLI users, however, this often means that a simple config mistake of setting the wrong num-units in the config file leads to weeks of postdata file initialisation being quickly wiped out.
With this change, the current default behaviour is preserved: redundant files are deleted. If the new option is enabled, an info entry is logged for redundant files, but they are not deleted.
This change prepares the stage for a dependent change in go-spacemesh.