Skip to content

Feature/nameservice#590

Merged
bplatz merged 11 commits intomainfrom
feature/nameservice
Oct 4, 2023
Merged

Feature/nameservice#590
bplatz merged 11 commits intomainfrom
feature/nameservice

Conversation

@bplatz
Copy link
Contributor

@bplatz bplatz commented Oct 4, 2023

This extracts the name service from the connection, to ensure support of keeping query servers' local dbs up to date with remote fluree/server instances. In addition to extracting the nameservice from the connection, it introduces:

  • sync? method - which when true on a nameservices, forces a synchronous nameservice update/write after a commit. Currently, if a commit writes successfully, however the nameservice fails, the commit is "orphaned", or worse. Every connection should have one synchronous nameservice.
  • Support for multiple nameservices on a single connection (e.g. for IPFS, you'd likely use IPNS [async] plus either local filesystem [sync] or something like Nexus)
  • A new -notify method on the ledger, where a nameservice can notify a ledger that an updated commit exists to keep its in-memory db current, avoiding the need for fluree/load

# Conflicts:
#	src/fluree/db/api/transact.cljc
#	src/fluree/db/conn/memory.cljc
#	src/fluree/db/conn/remote.cljc
#	src/fluree/db/json_ld/commit_data.cljc
#	src/fluree/db/json_ld/reify.cljc
#	src/fluree/db/ledger/json_ld.cljc
@bplatz bplatz requested a review from a team October 4, 2023 10:35
Copy link
Contributor

@zonotope zonotope left a comment

Choose a reason for hiding this comment

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

Looks good to me, although I'm still thinking through the ideas around
how nameservices relate to ledgers and chains of commits. In any case 👍🏾

@@ -0,0 +1,98 @@
(ns fluree.db.util.filesystem
Copy link
Contributor

Choose a reason for hiding this comment

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

This is similar to a lot of the work I was doing on my paused branch re-working our protocol surface and adding a storage protocol. I'm glad to see these changes to more isolate filesystem access, and I'm assuming we'll also use this ns for filesystem access in server too. I think there's still room to put all of this behind a general storage protocol to make memory access, ipfs access, s3 access, etc. more composable, but that can come later.

:branch (if branch
(util/keyword->str branch)
"main")
:ns (mapv #(if (map? %)
Copy link
Contributor

Choose a reason for hiding this comment

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

tying commits to nameservices feels a little weird to me, but maybe because i'm just not used to the new concepts. In my mind, a nameservice maintains a mapping from ledger name to the head of a chain of commits, and that chain of commits should be valid whether or not someone reading them has access to a particular nameserver, or even if that nameserver has disappeared.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Publishing a nameservice in a commit is just additional metadata, and actually should be optional. Here I've made it required, but I'd see us adding the ability to configure what (if any) nameservices get published in a commit.

I think most decentralized use cases should always point to a commit/db, never to a nameservice - as that is what a specific activity was based on at some moment in time.

If all I have is a commit however, it may be useful, or even critical, for some verification process to know if that data has been updated since. By having the nameservice(s) available in the commit, it is possible to see if the data publisher has subsequently updated information.

A good use case will be using a Fluree ledger to store your decentralized ID/DID. When using your DID, it is important for verification to know the public keys associated with it at the moment in time it was used to sign something. However, a verifier should also be able to consult the latest version to see if the public key was subsequently revoked because it was found to be compromised at the time it was used. Because the commit author supplied name service(s) in the commit, in this case the verifier can see what has changed since it was used and decide if those changes influence its opinion on its activity.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. this makes sense. I think my mind is still stuck in the blockchain world where once something is published, you can't change it, but we can't rely on that here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we have the best of both worlds. The commit is immutable and can stand on its own. Future updates are available if relevant/needed for the particular use case.

@@ -0,0 +1,41 @@
(ns fluree.db.method.remote.core
Copy link
Contributor

Choose a reason for hiding this comment

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

definitely not for this pr, but i think we should also eventually move all of the f.d.method.* related functionality behind a protocol.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't love the method name here as well, but originally used it for consistency with the DID spec. I considered changing it in this PR, but tried to minimize changes ... and still not convinced it is the wrong name either. I'd welcome anyone changing the naming, structure, protocol, etc... probably needs a bit of all of those things.

(-push [nameservice commit-data] "Pushes new commit to nameservice.")
(-subscribe [nameservice ledger-alias callback] "Creates a subscription to nameservice(s) for ledger events")
(-unsubscribe [nameservice ledger-alias] "Unsubscribes to nameservice(s) for ledger events")
(-sync? [nameservice] "Indicates if nameservice updates should be performed synchronously, before commit is finalized. Failure will cause commit to fail")
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels like the information about which nameservice(s) should be synchronous should live on the conn instead of the nameservices themselves. The same nameservice type could be sync for one conn but async on another.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does live on the conn. A conn can get passed nameservices, else it will default to the nameservice we think makes the most sense for the conn. I did hard-code in the :sync? parameter on some, but ideally a developer can construct this however they like. So what you describe is how it operates.

@bplatz bplatz merged commit 92c6545 into main Oct 4, 2023
@bplatz bplatz deleted the feature/nameservice branch October 4, 2023 20:33
Copy link
Contributor

@zonotope zonotope left a comment

Choose a reason for hiding this comment

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

I wrote this comment yesterday in response to this comment but it didn't go through because I didn't realize I had started another review and needed to also click the "Review changes" button. In any case, it's not a big deal either way.

Comment on lines +60 to +63
(let [sync? (ns-proto/-sync? ns)]
(if sync?
(<? (ns-proto/-push ns commit-data))
(ns-proto/-push ns commit-data))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is what i meant to refer to about the synchronous nameservers living on the conn instead of the protocol. Here were baking the behavior of the nameserver in relation to the connection in the nameserver protocol, and changing behavior outside the protocol based on the response of a protocol method.

I think a function called synchronous-nameservers or primary-nameservers that takes a conn and returns the list of nameservers that it needs to wait on, and a similar function that takes a conn and returns the list of nameservers that can be fire and forget makes more sense to me than calling a protocol method on each nameserver and doing a different thing based on what it returns.

That way the protocol only represents the generic behavior of a nameserver and the systems outside of the nameservers that rely on them don't leak their behavior into the nameserver itself. That will make them more modular.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So if I understand correctly, are you saying that instead of instantiating a conn with a list of nameservers as an option (one of which should be synchronous), that we have two different option parameters on the conn, one called something like :synchronous-nameserver and one :asynchronous-nameservers, and then enforce the sync behavior based on what bucket the nameserver was placed into? If so, that's a fairly simple change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. and then we wouldn't need the sync? method on the nameserver protocol and the protocol would be more generic. Information about whether the nameserver is synchronous is in relation to the conn, not independent of it.

Copy link
Contributor Author

@bplatz bplatz Oct 5, 2023

Choose a reason for hiding this comment

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

Another thought, if a generic nameservice (and its corresponding ns type/method) can just live in the commit as metadata, then it perhaps doesn't need to be "hard coded" into the conn. The conn can have a set of nameservices it knows, but the individual ledger/commit is configured to write to specific ones - one of which is "primary" and always synchronous.

In this case, if you attempted to transact where the latest commit's nameservices are ones you know you can update, it will succeed - else it will throw an exception without you first changing the commit nameservices - not unlike how we allow defaultContext to get updated from time to time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nameservices baked into the commit is still stretching my mental model and I'm still getting used to it, but I think I like this idea. It seems more flexible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants