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

feat: add dispatch count for list objects #1427

Merged
merged 26 commits into from
Mar 8, 2024

Conversation

poovamraj
Copy link
Contributor

Description

As a continuation of adding DispatchCount metrics to the Check endpoint, we have added it to ListObjects as well to identify the amount of reverse expands and checks that are done for each request.

ListObjects works by reverse expanding from a subject to a list of objects with particular relation. There will be recursion involved in cases union and inheritance uses cases. And we run the check on each object that is retrieved if there is intersection or exclusion present so as to identify the related list of objects. We are counting these dispatches to identify the complexity of queries.

References

Continuation of Dispatch Count in Check - #1343

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

@poovamraj poovamraj requested a review from a team as a code owner March 7, 2024 22:12
Copy link

codecov bot commented Mar 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (main@f27a553). Click here to learn what that means.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1427   +/-   ##
=======================================
  Coverage        ?   85.81%           
=======================================
  Files           ?       85           
  Lines           ?     7938           
  Branches        ?        0           
=======================================
  Hits            ?     6811           
  Misses          ?      793           
  Partials        ?      334           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

poovamraj and others added 2 commits March 8, 2024 14:23
poovamraj and others added 2 commits March 8, 2024 14:45
Co-authored-by: Jonathan Whitaker <jon.b.whitaker@gmail.com>
@jon-whit
Copy link
Member

jon-whit commented Mar 8, 2024

@poovamraj please add some more tests, specifically these cases:

Test Case 1

model
  schema 1.1

type user

type group
  relations
    define member: [user]
  • group:eng#member@user:jon
  • group:fga#member@user:jon

ListObjects(group#member, user:jon)

This returns 2 dispatches - note that this is non-ideal and points out an inefficiency in our algorithm, because user is directly related to group#member and so this should be a single database lookup without any dispatch required. The only time dispatch should be required is with expansion of usersets and/or nested group/hierarchy expansion. More on this coming soon 😄 . I'm working on alternative algorithm proposals to avoid this and be more efficient.

Test Case 2

model
  schema 1.1

type user

type group
  relations
    define member: [user, group#member]
  • group:eng#member@group:fga#member
  • group:fga#member@user:jon

ListObjects(group#member, user:jon)

This should return 2 dispatches today - note: it should only require 1 but because of the inefficiency in our algorithm today this returns 2.

Test Case 3

model
  schema 1.1

type user

type document
  relations
    define editor: [user]
    define viewer: editor
  • document:1#editor@user:jon

ListObjects(document#viewer, user:jon)

This should return 2 dispatches today - again, it should only require 1 dispatch but it won't until we improve our algorithm a bit.

pkg/server/commands/list_objects.go Show resolved Hide resolved
pkg/server/commands/list_objects.go Outdated Show resolved Hide resolved
pkg/server/commands/reverseexpand/reverse_expand.go Outdated Show resolved Hide resolved
poovamraj and others added 5 commits March 8, 2024 19:58
Co-authored-by: Maria Ines Parnisari <maria.inesparnisari@okta.com>
Co-authored-by: Maria Ines Parnisari <maria.inesparnisari@okta.com>
Co-authored-by: Maria Ines Parnisari <maria.inesparnisari@okta.com>
…enfga/openfga into dispatch_count_for_list_objects
@poovamraj
Copy link
Contributor Author

@jon-whit added the test cases except No.1 as it is already the first test case

@miparnisari miparnisari changed the title Dispatch count for list objects feat: add dispatch count for list objects Mar 8, 2024
poovamraj and others added 6 commits March 8, 2024 20:26
Co-authored-by: Jonathan Whitaker <jon.b.whitaker@gmail.com>
Co-authored-by: Jonathan Whitaker <jon.b.whitaker@gmail.com>
Co-authored-by: Jonathan Whitaker <jon.b.whitaker@gmail.com>
Copy link
Contributor

@willvedd willvedd left a comment

Choose a reason for hiding this comment

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

Nice job.

@miparnisari miparnisari merged commit 8c5f520 into main Mar 8, 2024
11 of 13 checks passed
@miparnisari miparnisari deleted the dispatch_count_for_list_objects branch March 8, 2024 20:49
@poovamraj poovamraj mentioned this pull request Mar 14, 2024
4 tasks
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

4 participants