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

When a Person has as a PersonRole in which the PersonPermission for the Person ClassKind is set to READ doing a GET on SiteDirectory has poor performance #339

Closed
4 tasks done
samatstariongroup opened this issue Mar 8, 2024 · 4 comments · Fixed by #340
Assignees
Milestone

Comments

@samatstariongroup
Copy link
Member

samatstariongroup commented Mar 8, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am running the latest version of the COMET Web Services
  • I have searched open and closed issues to ensure it has not already been reported

Description

In the case when there are many EngineeringModels available in a server and when each model has multiple Particpants, a READ on SiteDirectory has poor performance in the following case:

  • When a Person has as a PersonRole in which the PersonPermission for the Person ClassKind is set to READ doing a GET on SiteDirectory has poor performance

Steps to Reproduce

  • setup a server with the following
    • create a PersonRole called read that has for ClassKind.Person -> READ
    • create a PersonRole called modify that has for ClassKind.Person -> MODIFY
    • create a PersonRole called modifyownperson that has for ClassKind.Person -> MODIFY_OWN_PERSON
    • create 30 users with a distribution of the before mentioned personroles (10 per kind of PersonRole)
    • create 20 models and 30 participants in each model
  • Verify that performance of HTTP GET of the SiteDirectory?extent=deep has varying performance depending on the assigned rol

System Configuration

COMET Web Services version: 7.1.1

  • PostrgreSQL: 12.3
@antoineatstariongroup
Copy link
Contributor

antoineatstariongroup commented Mar 11, 2024

First investigation with the 2 defined configurations :

  • new = rheagroup/cdp4-database-community-edition:latest and rheagroup/comet-webservices-community-edition:8.0.0-rc25
  • old = rheagroup/cdp4-database-community-edition:2.0.0 and rheagroup/cdp4-services-community-edition:7.1.1

Associated dumps are contained in the following archive: (each db have setup explained in the issue but instead of having 20 models, there are 30 models)

cdp4db_dumps.zip

Here are results :

new_test_result.txt
old_test_result.txt

It seams the we do have a less performant login with the role MODIFY_OWN_PERSON but performance are way better with version 8.0.0-rc25

@antoineatstariongroup
Copy link
Contributor

antoineatstariongroup commented Mar 11, 2024

Investigation: new information;

The PermissionService for READ action, at SiteDirectory level can hit the database on multiple cases: if the PersonAccessRightKind is READ_IF_PARTICIPANT (not on the current scope) or MODIFY_OWN_PERSON.

On that last case, for each Person that is retrieved on the database, a new check is done against the database to check whether the person is a participant within a model that the logged person is also a participant.

@samatstariongroup
Copy link
Member Author

Investigation: new information;

The PermissionService, at SiteDirectory level can hit the database on multiple cases: if the PersonAccessRightKind is READ_IF_PARTICIPANT (not on the current scope) or MODIFY_OWN_PERSON.

On that last case, for each Person that is retrieved on the database, a new check is done against the database to check whether the person is a participant within a model that the logged person is also a participant.

@antoineatrhea that was my suspicion, i think the service is already registered as a per-request, so we can apply some kind of caching routinel Or we can do a more complex, less naive SQL statement in a DAO class to suit the needs and make it more performant.

@antoineatstariongroup
Copy link
Contributor

@samatrhea making BULK check would be more efficient : instead of checking one instance of each type, why not checking all instances of each type directly ? only one SQL statement would then be made (without any changes on current statement), just need to re-think the way that permission and filtering are done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants