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

Handle large user set #1800

Merged
merged 10 commits into from
Nov 8, 2017
Merged

Handle large user set #1800

merged 10 commits into from
Nov 8, 2017

Conversation

nathandunn
Copy link
Contributor

@nathandunn nathandunn commented Nov 8, 2017

  • handle searching
  • handle sorting
  • retest editing

@nathandunn nathandunn added this to the 2.0.8 milestone Nov 8, 2017
@nathandunn nathandunn self-assigned this Nov 8, 2017
@ghost ghost added the in progress label Nov 8, 2017
@nathandunn
Copy link
Contributor Author

@deepakunni3 This is ready for testing / review.

@childers
Copy link
Collaborator

childers commented Nov 8, 2017

@nathandunn what do you consider a large user set? 500, 1,000, 10,000?

@nathandunn
Copy link
Contributor Author

nathandunn commented Nov 8, 2017 via email

@epaule
Copy link
Contributor

epaule commented Nov 8, 2017

first impression (with 1000 users and 1001 organisms):

  • the rest API to add users is still very slow (actually it becomes exponentially slower with the number of users already added)
  • the user web interface is working smoothly and paginating properly without any problems

@nathandunn
Copy link
Contributor Author

nathandunn commented Nov 8, 2017 via email

@deepakunni3
Copy link
Member

@nathandunn Tested and looks good. 👍
Didn't have any issues with the API, but that could be because I am adding users to a somewhat fresh database.

@nathandunn
Copy link
Contributor Author

nathandunn commented Nov 8, 2017 via email

@nathandunn nathandunn merged commit d5cb6de into GMOD:master Nov 8, 2017
@ghost ghost removed the in progress label Nov 8, 2017
@khowe
Copy link

khowe commented Nov 8, 2017

We are making 3 specific call to the API for each user:

  1. user/createUser and user/loadUsers (to create the new user, and check that the user actually ended up in the database
  2. organism/addOrganism and organism/findAllOrganisms (to create a personal genome for that user, and check that the organism actually ended up in the db)
  3. user/updateOrganismPermission (to give the new user permission to READ/WRITE the new organism).

With this PR, we are finding (1) to now be fast. However, (2) is very slow, and we are getting timeouts. Could it be that the re-engineering you've done for Users also needs to be done for Organisms? Just a thought.

@nathandunn
Copy link
Contributor Author

nathandunn commented Nov 8, 2017 via email

@nathandunn
Copy link
Contributor Author

@khowe @epaule with the PR #1801 if you call organism/addOrganisms add the flag: "returnAllOrganisms:false" and it should speed things up quite a bit (otherwise it becomes a very LARGE transaction).

When you call "findAllOrganisms" you should be able to (already) provide the an "organism" key / value flag to filter out the return using the commonName or the ID.

@khowe
Copy link

khowe commented Nov 8, 2017

Thanks @nathandunn . We're already using the organism key with findAllOrganism, but the returnAllOrganisms:false flag on addOrganism will certainly help! Would there be any chance of adding that parameter for user/createUser too?

@nathandunn
Copy link
Contributor Author

@khowe user/createUser should not have this problem (unless you want to add the option to add the full user list with a flag).

@khowe
Copy link

khowe commented Nov 8, 2017

Ah yes @nathandunn , you're right.

I was actually thinking of user/loadUsers. You can currently restrict the response to a single user with the userId parameter. But it would be useful for us to have an additional parameter to make it only include an organism in the organismPermissions array if the organism has non-empty permissions. e.g. "omitEmptyOrganisms:true" (or something like that).

@nathandunn
Copy link
Contributor Author

nathandunn commented Nov 8, 2017 via email

@nathandunn nathandunn mentioned this pull request Nov 8, 2017
@nathandunn
Copy link
Contributor Author

nathandunn commented Nov 8, 2017 via email

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

5 participants