Skip to content

Conversation

@psrok1
Copy link
Member

@psrok1 psrok1 commented Oct 11, 2023

Your checklist for this pull request

  • I've read the contributing guideline.
  • I've tested my changes by building and running the project, and testing changed functionality (if applicable)
  • I've added automated tests for my change (if applicable, optional)
  • I've updated documentation to reflect my change (if applicable)

What is the current behaviour?

  • Object.access() eagerly loads all parents to include in the original relationship only these items that are accessible for currently logged user
  • This works awfully slow if accessed object has lots of parents

What is the new behaviour?

  • Added exclude_relations flag to GET /<object_type>/<object_dhash> endpoints that excludes relations from query result, loading only basic properties, attributes and tags.
  • Object.access() no longer loads parents so they're lazy-loaded (or eagerly loaded explicitly) and are not filtered by permissions. Internal code doesn't rely on that, but it may affect plugins

TODO
Right now we have breaking change because POST /<object_type>/<object_dhash> doesn't return relations info. Should we add another flag for that?

Test plan

Permission checks should be already covered by current tests

@psrok1 psrok1 force-pushed the feature/exclude-relations-on-object-get branch from 796c25d to 7863c0f Compare October 11, 2023 16:50
@yankovs
Copy link
Contributor

yankovs commented Nov 23, 2023

Hey! :)
Great work and this is a very welcome feature. We actually struggle with this exact issue since we have samples/configs that are 2 years old at this point in MWDB and have many connections and API requests on them result in timeout (this affects places such as karton-mwdb-reporter).

How do you think this change will affect MWDB? Will it make the parents/children view just not render? If it works like that it sounds reasonable to me. From my experience with a pretty big MWDB instance places like parents/children and Karton analysis box are the main hotspots in the view and I think worth considering lazy loading either via an explicit request (like the relations graph) or via pagination (which I know happens in children box, but not sure if it truly keeps an iterator and lazy loads).

Regarding breaking changes in POST, I think either way is fine honestly. I think relying on relationships that return from post request is a pretty niche use case and I believe that if MWDBlib has a way to specify a flag to give relations just like you mentioned it would be sufficient for users of the system.

@psrok1 psrok1 marked this pull request as draft July 12, 2024 09:26
@yankovs
Copy link
Contributor

yankovs commented Feb 18, 2025

Bumping!

@psrok1
Copy link
Member Author

psrok1 commented Mar 4, 2025

Hey @yankovs. First I'm going to introduce some limiting in #1023, I think this would be less intrusive.

From my experience with a pretty big MWDB instance places like parents/children and Karton analysis box are the main hotspots in the view

I agree that another hotspot is Karton analysis list and we need to rework it a bit as well 🤔

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.

3 participants