Do not require the mithril client to create the DB directory

[TrevorBenson]

Why

When mithril-client CLI is instructed to snapshot download it expects to be able to create the db directory inside the --download-dir which points to the parent directory instead of the db directory where the download actually occurs. When the db directory already exists it results in an error.

 Error: Unpack directory '/cardano/db' already exists, please move or delete it.

At first glance this seems simple to resolve, just remove the directory. However, this is opinionated behavior limits the node operators choices and can lead to unnecessary complications, changes to file system layouts, even failures in some designs.

When a node operator:

• Provides a dedicated partition for the db directory and the path is a mount point.
• Uses:
  • Docker/Podman and uses a container volume mounted to the db directory
  • Kubernetes (k8s) and uses a Persistent Volume (PV) mounted to the db directory

For a basic POC of the issue a bare metal reproduction scenario is provided:

Steps to reproduce

1. Create a partition and format filesystem, etc.
2. Mount the filesystem on /cardano/db
3. Run mithril-client --aggregator-endpoint $AGGREGATOR_ENDPOINT snapshot download --genesis-verification-key $GENESIS_VERIFICATION_KEY --download-dir /cardano
4. Observe error Error: Unpack directory '/cardano/db' already exists, please move or delete it.
5. Try and remove the directory rmdir /cardano/db
6. Observe error rmdir: failed to remove '/cardano/db': Device or resource busy

While this is only bare metal, in most of the mentioned cases it is essentially not viable to simply rmdir /path/to/db and let the Mithril client create the directory. While at first glance this may seem simple, just make the mount point the parent directory, this leads to other complications when the db directory or it's parent directory:

1. Is the root file system
2. Contains one or more separate mount points
3. Is a Docker (container) volume
4. Is a persistent volume (k8s PV)

Workarounds to these issues don't only result in unwanted file system layouts, but can result in requiring additional IO after the download to relocate the data when separate file systems are required, which can also lead to double the required storage being available. Separate than this it can create unwanted dependency relationships which eventually lead to other issues.

I filed this as a feature request, but the opinionated nature of this design choice I feel comes close to what I would define as a bug or flaw in the design due to the unnecessary requirements it may impose on node operators integrating mithril into existing designs which may have been in use for multiple years without changes.

What

• Do not require the db directory to be created by the mithril client to download, just for it to be empty.

This could be achieved just by removing the requirement to create the directory when it is empty. Or having the --download-dir target the actual db directory instead of its parent directory, and is required to be empty.

[jpraynaud]

Thanks @TrevorBenson for creating this issue.

In order to avoid breaking changes in the client CLI, we could also support a new option like --allow-empty-download-dir or --skip-create-download-dir that would allow the behavior that you expect?

[TrevorBenson]

Thanks @TrevorBenson for creating this issue.

In order to avoid breaking changes in the client CLI, we could also support a new option like --allow-empty-download-dir or --skip-create-download-dir that would allow the behavior that you expect?

Yes, additional parameters would be acceptable to achieve the expected behavior.

[Alenar]

Hi @TrevorBenson, thanks for the issue.

After rethinking a bit about this issue as a team, we decided to just accept existing, but empty, db directory (if the directory doesn't exist it will be created, same behavior as before). No extra parameter needed.

This behavior is implemented in #1581 that we can merge as soon as today if it's alright for you.

[TrevorBenson]

Hi @TrevorBenson, thanks for the issue.

After rethinking a bit about this issue as a team, we decided to just accept existing, but empty, db directory (if the directory doesn't exist it will be created, same behavior as before). No extra parameter needed.

This behavior is implemented in #1581 that we can merge as soon as today if it's alright for you.

This sounds perfectly acceptable.

Thank you