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

settings: Maintain a cache/index for assignments by role #9363

Closed
wants to merge 1 commit into from

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Jun 12, 2024

Description

In order to be able to improve the speed of listing all assignments for a specific role, we now maintain a cache of all assignments per role.

The cache implementation is mainly copied from reva's sharecache for the jsoncs3 share manager. It is stored in the metadata service.

Related to: #8938

There is still quite a few things left to do here:

  • There is some interference with the ttl based metadata cache that is already in place for the role assignments. The metadata cache caches the results of calls to e.g. SimpleUpload, SimpleDownload and ReadDir. As it is purely ttl base it will cause inconsitencies with the etag based role cache, especially when multiple instances of the settings service are running. It's unclear to me yet how to address this without harming the ListRoleAssignments (by user) performance by just disabling the metadata cache.
  • As the initial role assignments are done via direct uploads to the metadata service (see e.g.:
    err = mdc.SimpleUpload(ctx, assignmentPath(accountUUID, ass.Id), b)
    ) the cache is not popluated correctly for the default assignments
  • There is no migration, or fallback to recreate the cache when starting this on a system which was created before this PR
  • Maintaining the index cause performance problems on it's own. With every WriteRoleAssignment or DeleteRoleAssignment call the cache file for the affected roles needs to be completely rewritten (including the marshalling/unmarshalling from/to JSON). Appart from creating issues with file versions (Metadata storage creates file revision for every upload #9282) this can be quite expensive with increasing numbers of assignements for a specific role. (On my NFS backed test system creating 1000 users with the default user role assignement already became almost unbearable slow after about 500 users, including sporadic failures)

On the positive side of things, this improvements for lookup of assignments by roleid are quite visible. The $filter=(appRoleAssignments/any(m:m/appRoleId eq 'd7beeea8-8ff4-406b-8fb6-ab2dd81e6b11'))&$orderby=displayName&$expand=appRoleAssignments query is now down to less than a second (from 14s). Same setup as in #9326.

Copy link

update-docs bot commented Jun 12, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@mmattel
Copy link
Contributor

mmattel commented Jun 13, 2024

@rhafer are you planning to (re)use our existing cache implementation(s)? If yes, we need to add some info in the settings readme as we have with other services using caches.

@rhafer
Copy link
Contributor Author

rhafer commented Jun 14, 2024

@rhafer are you planning to (re)use our existing cache implementation(s)? If yes, we need to add some info in the settings readme as we have with other services using caches.

The settings service is already using the existing cache implementation today. And AFAICS has the info in the readme.

The additional cache (which is more of an index actually) introduced in this PR, wouldn't need any additional configuration.

In order to be able to improve the speed of listing all assignments for
a specific role, we now maintain a cache of all assignments per role.

The cache implementation is mainly copied from reva's sharecache for the
jsoncs3 share manager. It is stored in the metadata service.

Related: owncloud#8938
@rhafer rhafer force-pushed the issue/8938-roleIdIndex branch from 495fa54 to 38df6d8 Compare June 14, 2024 07:05
Copy link

sonarcloud bot commented Jun 14, 2024

@rhafer
Copy link
Contributor Author

rhafer commented Aug 21, 2024

Closing this for now. This approach didn't really scale well when there are a lot of assignments for a single role. (Changing a single assignment requires a full upload/download cycle on the metadata backend for the change index files. Which will get larger as the number of users increases.)

@rhafer rhafer closed this Aug 21, 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.

2 participants