I don't see an issue for having a storageRoot configuration option in the consensus entry of the config map when the protocol is "raft" that points to a directory that may or may not be the same as the root directory the connection uses

Raft has two directories - one being for raft stuff (logs, snapshots) - currently: consensus.logDirectory and the other is stuff needed for the state machine and whose files are only ledger files: consensus.ledgerDirectory - which performs the function of storageRoot you mentioned above.

The issue is if connection.storagePath != consensus.ledgerDirectory your system will not work - so the fact that we have both right now will create issues for people if they are not careful - and of course why have both if not needed.

I think the easiest path might be to just copy this connection setting (connection.storagePath) to the raft config before calling ig/expand. That way we don't need to add any extra parameters or methods to the existing connection to accomplish this.

I don't see an issue for having a storageRoot configuration option in the consensus entry of the config map when the protocol is "raft" that points to a directory that may or may not be the same as the root directory the connection uses

Raft has two directories - one being for raft stuff (logs, snapshots) - currently: consensus.logDirectory and the other is stuff needed for the state machine and whose files are only ledger files: consensus.ledgerDirectory - which performs the function of storageRoot you mentioned above.

The issue is if connection.storagePath != consensus.ledgerDirectory your system will not work - so the fact that we have both right now will create issues for people if they are not careful - and of course why have both if not needed.

I think the easiest path might be to just copy this connection setting (connection.storagePath) to the raft config before calling ig/expand. That way we don't need to add any extra parameters or methods to the existing connection to accomplish this.

Then this sounds like the case I mentioned above where we'd using the storage protocol to allow the raft component to access the ledger files it needs to.

The files that raft accesses using consensus.ledgerDirectory should instead be accessed using the storage api if I understand correctly. We can do this by making the :fluree/connection component a dependency of the :fluree/raft component, and any raft code that needs to access ledger files should use the storage record connected to the connection with the fluree.db.storage/* api.

Eventually, we should have a dedicated :fluree/storage component that is a dependency of both the :fluree/connection and :fluree/raft components, but we'll need to finally finish the connection refactor that I have been mentioning in order to make that possible.

The files that raft accesses using consensus.ledgerDirectory should instead be accessed using the storage api if I understand correctly. We can do this by making the :fluree/connection component a dependency of the :fluree/raft component

This is what is there now, and caused the issue. Here is the file store write method: https://github.com/fluree/db/blob/064d63dd28ea26519c501e1090b8ad18adb58b27/src/clj/fluree/db/storage/file.cljc#L26-L44

You'll see it does a bunch of stuff, none of which is needed for consensus and it actually breaks consensus. There is only one line in all of that which is needed, and that is:
(<? (fs/write-file absolute bytes))

So we could still use the store protocol, but will need a new method that doesn't do all of these things (this is option #3 I mentioned in the original post).

The files that raft accesses using consensus.ledgerDirectory should instead be accessed using the storage api if I understand correctly. We can do this by making the :fluree/connection component a dependency of the :fluree/raft component

This is what is there now, and caused the issue. Here is the file store write method: https://github.com/fluree/db/blob/064d63dd28ea26519c501e1090b8ad18adb58b27/src/clj/fluree/db/storage/file.cljc#L26-L44

You'll see it does a bunch of stuff, none of which is needed for consensus and it actually breaks consensus. There is only one line in all of that which is needed, and that is: (<? (fs/write-file absolute bytes))

So we could still use the store protocol, but will need a new method that doesn't do all of these things (this is option #3 I mentioned in the original post).

sorry, I didn't understand the problem, but I think I get it now. In this case, I think we want option 4: another protocol because this defines different behavior that not all implementations will have (like ipfs for example).

The current fluree.db.storage/Store protocol could be renamed to fluree.db.storage/ContentAddressedStore, and then we have a new fluree.db.storage/ByteStore (or a better name) that will have read-bytes and write-bytes methods (along with any other methods we might need).

Then, all the current storage records would still implement the ContentAddressedStore protocol, and at least FileStore would also implement the ByteStore protocol. That way the two protocols could both have access to the same storage directory without breaking encapsulation.

I think this would solve the issue if I understand it correctly.

I think this would solve the issue if I understand it correctly.

Yes it would, but feels a little heavy. The only thing we need for now is a write function... everything else the conn handles. We only need to call this: (<? (fs/write-file absolute bytes)) -- which itself isn't really much more than clojure.java.io/write.

The only thing we are missing is the first argument - absolute as we need the configured storage path. If we could just get that path from (a) the config or (b) the conn we'd be all set. I did (b) in this PR, because there is no way to get (a) currently.

I actually am increasingly wondering if we want to move the base storage path out of the connection config - but it would still use it. But so would a local nameservice that might use the file system, and of course per this PR so would the consensus protocol. That is an option that solves (a) - and I'm thinking we might need to solve that anyhow because if we maintain a local NS, even if using IPFS conn, we'll need a base path.

feels a little heavy

I agree that it's heavier than just making the absolute path available, but making the path available outside of the storage protocol and writing to it directly with different code breaks the encapsulation of the protocol and could lead to issues with keeping the implementation in sync further down the line.

Having another protocol that the same record can implement preserves the abstraction of that record which helps keep the protocol implementations in sync over time, and gives us more flexibility in what we can do here in the future with less fears of breaking existing functionality.

@zonotope I ended up implementing a :server config component that contains :storage-path. This really should be a server-level setting, not just for connections - but also raft and nameservice.

This solves the encapsulation problem you mentioned on the connection, and we needed to add a new server-level config map anyhow for zero-trust setting, admin-pub-key, etc. and this will get used for more than just this one setting shortly.

All these changes are in the latest commit: 68e2dbd

@zonotope I ended up implementing a :server config component that contains :storage-path. This really should be a server-level setting, not just for connections - but also raft and nameservice.

This solves the encapsulation problem you mentioned on the connection, and we needed to add a new server-level config map anyhow for zero-trust setting, admin-pub-key, etc. and this will get used for more than just this one setting shortly.

All these changes are in the latest commit: 68e2dbd

How does this impact decoupling the component types and moving to generic connections as we've been discussing? Would we still be able to use, say, both an s3 nameservice along with an ipns nameservice in tandem, ipfs storage for commits, file storage for indexes, and control all of that from the configuration map alone?

It doesn't feel like a server level config because where we store commits, transactions, indexes, and name service records don't have to be in the same path if these artifacts are always accessed through protocols. They don't even have to use the same storage substrate.

How does this impact decoupling the component types and moving to generic connections as we've been discussing? Would we still be able to use, say, both an s3 nameservice along with an ipns nameservice in tandem, ipfs storage for commits, file storage for indexes, and control all of that from the configuration map alone?

In all of these cases you'd have a local nameservice on disk which is what the server directly relies on. This is already what Nexus does for IPFS - it uses both IPNS but has a local FS nameservice that combines both IPFS ledger and local/filesystem ledgers. The local NS is the "authority", it can sync/pull with any number of remote nameservices.

It doesn't feel like a server level config because where we store commits, transactions, indexes, and name service records don't have to be in the same path if these artifacts are always accessed through protocols. They don't even have to use the same storage substrate.

Yes, but in all cases the local NS record for the respective ledgers will point to where the files, indexes, etc. are (which can be anywhere). There is no reason I can think of why you'd ever have, or want to maintain, multiple local directories for the same fluree server instance. This is actually one of the things that broke Raft - that it was being maintained in two spots and if you didn't configure the identical value for both it wouldn't work. It sounds like that problem multiplied - thats a lot of stuff to get wrong for no value add.

I'd like Fluree to just use the local storage directory I give it and do its thing - how Fluree uses that directory is mostly up to it. Optionally I may specify a different log directory as often the former will use block storage and the latter will use local drives. I can't think of a use case where that wouldn't work or be insufficient - and I don't see how that would impact any of the things you mentioned above either about using S3, IPFS etc.

There is a new branch that will address the store protocol suggestions above.