Skip to content

Watcher and subscription protocols #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 38 commits into from
Jan 13, 2025
Merged

Watcher and subscription protocols #104

merged 38 commits into from
Jan 13, 2025

Conversation

zonotope
Copy link
Contributor

@zonotope zonotope commented Jan 8, 2025

This patch adds protocols for watcher and subscription functionality to allow us to more flexibly publicize new commits as well as report on the status of pending transactions.

This patch also adds a new type of standalone transactor that does not terminate it's return channel until the transaction is fully processed instead of maintaining a buffer of pending transactions. This new transactor will apply back pressure to down stream callers instead of relying on those callers to detect and manage their submitted transactions from errors returned.

This patch lastly removes the commit and data files from consensus messages in favor of just including the commit address so receivers can download the files themselves from storage as necessary. This temporarily breaks Raft consensus, but this will be fixed in a subsequent patch.

Lastly, this patch refactors the application route initialization to allow us to (eventually) initialize routes more dynamically, and adds a callback route to allow delegated transactors to report the status of pending transactions they're processing.

@zonotope zonotope requested a review from a team January 8, 2025 20:49
@zonotope zonotope self-assigned this Jan 8, 2025
(fluree/commit! ledger staged-db {:file-data? true}))
broadcast-msg (events/transaction-committed params commit-result)]
result (deref!
(fluree/apply-stage! ledger staged-db))
Copy link
Contributor

Choose a reason for hiding this comment

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

What is apply-stage do here? When I try to load it my repl it says no definitions found.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I found the branch, I'm not sure why xref wasn't working for me. So it looks like apply-stage is just commit with a wrapper.

I'm kind of ambivalent about having this as a top-level api - we call directly into the connection ns in other places, could we do that here too instead of making apply-stage top-level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to be a top level api because it was part of the commit! function. You had to pass the :file-data? true option to get the output to have this shape. I don't think we should be doing this kind of control flow with options, so I split that functionality up into two separate functions. I made it a top level api because that functionality used to be a top level api, and I can see other users wanting to know the commit address after calling the commit.

; :ledgers {"some/ledger" {}} ;; <- map where each key is a ledger-id subscribed to
; :did nil}}
; :closed? false}
(defprotocol Publicizer
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 minor, but I feel like this "publishes" rather than "publicizes" and would maybe be better with the name Publisher, publish-commit, and publish-new-ledger. I don't insist on it, though : )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We already have a Publisher protocol that the name services implement, so I didn't want to reuse that term.

Copy link
Contributor

@dpetran dpetran left a comment

Choose a reason for hiding this comment

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

🗞️

@zonotope zonotope merged commit 8d94fdc into main Jan 13, 2025
5 checks passed
@zonotope zonotope deleted the feature/integrated-server branch January 13, 2025 17:03
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