Skip to content
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

Dispatch count to resolution metadata #1343

Merged
merged 22 commits into from
Feb 28, 2024
Merged

Dispatch count to resolution metadata #1343

merged 22 commits into from
Feb 28, 2024

Conversation

willvedd
Copy link
Contributor

@willvedd willvedd commented Feb 7, 2024

Description

What makes a check query expensive in both resources and time? Ostensibly, a high number of datastore queries is probably the first answer. However, that isn't the full picture and there are other aspects of a check query that could affect the cost. For example, the depth of the relationships, the number of tuples, the number of dispatches, etc. We have decent intuitions but ultimately we need data to understand the relationships between those facets and the bottom-line performance.

This PR is a first-pass at trying to unearth some check resolution metrics through metadata so that they can be reported on and analyzed.

References

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@willvedd willvedd marked this pull request as ready for review February 21, 2024 17:43
@willvedd willvedd requested a review from a team as a code owner February 21, 2024 17:43
@jon-whit
Copy link
Member

jon-whit commented Feb 22, 2024

@willvedd would you mind adding this test as well? The gist just exercises direct userset dispatches and is a good test to have. It's a little easier to reason about the direct userset lookups and their dispatches.

Side note for future discussion and work, the test case(s) in the gist are a good example of something we can optimize in our Check algorithm by doing direct "bulk" lookups first, and then dispatching residual subtrees. For example, when we have to evaluate the subtree for group:eng#member@user:jon we find 3 usersets: group:1#member, group:2#member, and group:3#member. user:jon could be a direct member of any of those sets because of the type restrictions, and we probably shouldn't have to dispatch those subproblems individually to find that out. Instead, we could do a single dispatch that does a direct lookup of the form SELECT COUNT(..) FROM tuple WHERE object_type="group" AND object_id IN ["1", "2", "3"] AND user_object_type="user" AND user_object_id="jon" AND user_object_relation="". If we don't find any terminal relationships (e.g. the query above returns no results), then we can dispatch the residual rewritten subproblems (if any - none in this example though). Doing this would reduce 4 potential dispatches to always 2 dispatches in scenarios such as this. The less dispatches the better.

@willvedd
Copy link
Contributor Author

Note: Only added dispatch count metrics for check for now. Listobjects will require a follow-up PR once this is approved.

tests/check/check_test.go Outdated Show resolved Hide resolved
willvedd and others added 3 commits February 28, 2024 08:37
Co-authored-by: Jonathan Whitaker <jon.b.whitaker@gmail.com>
jon-whit
jon-whit previously approved these changes Feb 28, 2024
@willvedd willvedd merged commit 4fd659e into main Feb 28, 2024
11 of 13 checks passed
@willvedd willvedd deleted the dispatch-metrics branch February 28, 2024 19:28
@miparnisari miparnisari mentioned this pull request Mar 1, 2024
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.

None yet

2 participants