[Feature] Filesystem reorganization #4033
Conversation
kaimast
force-pushed
the
feat/filesystem-reorg
branch
from
6e62037 to
66e9415
Compare
I think documentation in the snarkOS README, release and release blog should be sufficient.
I think this may not reflect how important it is for validators to keep
Given that backups are essentially required are for validators, what about we make it a mandatory flag for |
kaimast
force-pushed
the
feat/filesystem-reorg
branch
from
66e9415 to
ddda90a
Compare
kaimast
force-pushed
the
feat/filesystem-reorg
branch
2 times, most recently
from
eef8c59 to
601266f
Compare
kaimast
force-pushed
the
feat/filesystem-reorg
branch
3 times, most recently
from
4a27527 to
7ce27dc
Compare
kaimast
force-pushed
the
feat/filesystem-reorg
branch
from
7ce27dc to
743f6df
Compare
| /// That folder may contain sensitive data, such as the JWT secret, and should not be shared with untrusted parties. | ||
| /// For validators, it also contains the latest proposal cache, which is required to participate in consensus. | ||
| #[clap(long, verbatim_doc_comment)] | ||
| pub node_data: Option<PathBuf>, |
There was a problem hiding this comment.
Revisiting a naming discussion, what about we name the flags legder_storage and node_data_storage. We keep storage as alias but hide it from documentation where possible and we can deprecate it in the future.
| Ok(std::fs::write(jwt_secret_path, token)?) | ||
| }; | ||
| if self.node_data.is_some() && !matches!(storage_mode, StorageMode::Custom(_)) { | ||
| warn!("Custom path set for `--storage`, but not for `--node-data`. The latter will use the default path."); |
There was a problem hiding this comment.
Shall we just enforce via Clap that both flags are either set or not set together? Node operators will miss this log and not be aware of the implications.
There was a problem hiding this comment.
Correction: only make it required for validators. So I guess we can stick with the current if statements.
There was a problem hiding this comment.
Yeah, I was not sure if it makes sense to only specify a custom path for one in some cases. The warning is just out of additional caution.
There was a problem hiding this comment.
Why did you resolve this? I suggested we make the flags depending on each other for validators and do not see argumentation
| } | ||
|
|
||
| // Parse the node data directory. | ||
| let node_data_dir = parse_node_data_dir(&self.node_data, N::ID, self.dev).with_context(|| "Failed to setup node configuration directory")?; |
There was a problem hiding this comment.
The ledger variable is called storage_mode. Can we make the naming and for the two different folders more consistent?
Should/could we use another instance of StorageMode in NodeData?
There was a problem hiding this comment.
They are slightly different. storage_mode is eventually passed to aleo_std::aleo_ledger_dir to retrieve the ledger path. NodeDataDir already contains the node data path.
There was a problem hiding this comment.
Can you not resolve comments when there's potential disagreement?
Would it be possible to re-use StorageMode and to add another node_data method in the aleo-std repo instead? It would make reading and maintaining the code a lot easier.
|
Note this may need a minor merge to include #4054 once it's ready |
Fixes #4013.
This PR adds a new
node-datadirectory that contains all node-specific data (essentially everything but the ledger). This is located at~/.aleo/storage/node-data-{network_id}for production or./node-data-{network_id}-{dev}for development nodes. Alternatively, users can also pass a value to--node-dataand store the data at a custom path.At startup, a node will check for any files that indicate that the old storage format is used. In that case, a message is printed in how to migrate, and the node will not start. Additionally, the FAQ in the README contains information on how to migrate.
Alternatively, nodes can also be started with
--auto-migrate-node-datato do this automatically. This is not enabled by default out of extra precaution.