DCJ-708: Refactor query performance #2412
Conversation
| void testCancelDarCollectionAsChair_ChairHasDatasets() { | ||
| User user = new User(); | ||
| user.setEmail("email"); | ||
| user.setUserId(RandomUtils.nextInt(1, 10)); |
There was a problem hiding this comment.
why a random user id for testing?
There was a problem hiding this comment.
why a random user id for testing?
The only reason is to have something to query on. The actual value is irrelevant.
| * @return List of Dataset IDs | ||
| */ | ||
| public List<Integer> findDatasetIdsByDACUser(User user) { | ||
| return datasetDAO.findDatasetIdsByDACUserId(user.getUserId()); |
There was a problem hiding this comment.
why change from user email to user id?
There was a problem hiding this comment.
The original code was using string comparison which can sometimes be flaky based on casing, etc. In theory, both should work, but I feel like the integer primary key is more stable than than the email.
There was a problem hiding this comment.
Yes, typically you want to query by the integer primary key because it will be indexed, which makes for fast searches.
rjohanek
left a comment
rjohanek
left a comment
There was a problem hiding this comment.
this looks great! so much more efficient!
| * @return List of Dataset IDs | ||
| */ | ||
| public List<Integer> findDatasetIdsByDACUser(User user) { | ||
| return datasetDAO.findDatasetIdsByDACUserId(user.getUserId()); |
There was a problem hiding this comment.
Yes, typically you want to query by the integer primary key because it will be indexed, which makes for fast searches.
Addresses
Ticket: https://broadworkbench.atlassian.net/browse/DCJ-708
Summary
In this PR, we fix a long-running query bug that caused the instance to crash. This manifested in a never-ending loading screen for the user and an eventual redirect back to their console with an error message. The root of the problem is that it is very expensive to populate a large list of datasets which caused OOMs and the pod to crash/restart.
Fixes:
Have you read CONTRIBUTING.md lately? If not, do that first.