Skip to content

Fix/federated graph crawl#744

Merged
zonotope merged 61 commits intomainfrom
fix/federated-graph-crawl
Mar 29, 2024
Merged

Fix/federated graph crawl#744
zonotope merged 61 commits intomainfrom
fix/federated-graph-crawl

Conversation

@zonotope
Copy link
Contributor

This patch add support for selecting and formatting subgraphs rooted from subject nodes in federated queries. It required a reorganization of our graph crawl code to run in parallel so that we could execute the queries on each component of the federated graph, and stitch the results together with each recursive search.

I also added support for fuel tracking to the graph crawling code and activated the fuel tracker during the normal query execution, but we still have to turn it on both for history and simple subject crawl queries.

I also made a few ergonomic changes to make the index reading and dataset internal apis easier to use.

zonotope added 30 commits March 13, 2024 03:26
We'll eventually need to resolve references in a combined dataset, and not just
a single database, so I've isolated this step to allow for refactoring later.
These were never read. We definitely need fuel tracking for this component, but
we should hook in to the same fuel tracking system used by queries and
transactions. A few more architectural changes have to happen first before
that's possible.
@zonotope zonotope requested a review from a team March 22, 2024 04:32
@zonotope zonotope self-assigned this Mar 22, 2024
This test breakage has nothing to do with the work on this branch and was only
coincidentally passing before. I have marked it pending for now so as not to
hold up merging this branch until we fix it.
async/merge)
(match-tuple active-graph fuel-tracker solution pattern filters error-ch))))
(match-tuple active-graph fuel-tracker solution pattern filters error-ch))
(go)))
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the deal with this go call? I've not seen something like that before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, is it just meant to return a channel with nothing in it, because the other if branches return a channel? That makes sense, though maybe you could help me understand when we wouldn't have an active graph.

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 just changed it to reuse the same closed channel so as not to create any unnecessary go blocks. not a big deal if we did, but there's no need to in this situation. this is just the equivalent of nil in the async execution environment.

We don't have an active graph in the case where a query specifies a from-named clause but no from clause, and the particular where clause pattern does not specify a named graph. We used to throw an inscrutable error, but it actually just means there's no data to query. We could throw another error during validation to catch this situation, and I went back and forth over whether we should do that, but I think that's outside the scope of this work and doesn't match with how we treat other situations like this: if there's no data to query, we return an empty set.

(resolve-references ds cache context compact-fn select-spec current-depth fuel-tracker error-ch)
(append-id ds iri select-spec cache compact-fn error-ch)))))

(defn format-subject-flakes
Copy link
Contributor

Choose a reason for hiding this comment

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

Love the new name and the refactor of this whole namespace 🥇

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 ce981c9 into main Mar 29, 2024
@zonotope zonotope deleted the fix/federated-graph-crawl branch March 29, 2024 16:52
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