Bug #11803
repositories#get_all_permissions API should not be so slow
100%
Subtasks
Associated revisions
History
#1
Updated by Tom Morris almost 5 years ago
- Target version set to 2017-06-21 sprint
- Story points set to 1.0
#2
Updated by Tom Clegg almost 5 years ago
- Assigned To set to Tom Clegg
#3
Updated by Tom Clegg almost 5 years ago
- Status changed from New to In Progress
#5
Updated by Tom Clegg almost 5 years ago
It looks like user.group_permissions()
is the thing that is either cached or slow. Instead of calling/retrieving this for each user individually for each link giving permission to a repo, we can get one big hash up front with permissions for all users, so the inner loop needs to do 2 hash lookups.
There's certainly more room for improvement in get_all_permissions, but this easy change should bring the slowest queries down from ~60s to ~1s.
11803-repo-perms @ ec7510c680ee2065d6372fef6a340ef754dbe724
#6
Updated by Peter Amstutz almost 5 years ago
Tom Clegg wrote:
It looks like
user.group_permissions()
is the thing that is either cached or slow. Instead of calling/retrieving this for each user individually for each link giving permission to a repo, we can get one big hash up front with permissions for all users, so the inner loop needs to do 2 hash lookups.There's certainly more room for improvement in get_all_permissions, but this easy change should bring the slowest queries down from ~60s to ~1s.
11803-repo-perms @ ec7510c680ee2065d6372fef6a340ef754dbe724
LGTM.
But I got these test failures:
1) Failure: ContainerTest#test_find_reusable_with_logging_disabled [/usr/src/arvados/services/api/lib/log_reuse_info.rb:13]: unexpected invocation: #<ActiveSupport::Logger:0x564654464998>.info('find_reusable: starting with 12 container records in database') unsatisfied expectations: - expected never, invoked once: #<ActiveSupport::Logger:0x564654464998>.info(any_parameters) 2) Failure: JobTest#test_find_reusable_without_logging [/usr/src/arvados/services/api/lib/log_reuse_info.rb:13]: unexpected invocation: #<ActiveSupport::Logger:0x564654464998>.info('find_reusable: starting with 35 jobs readable by current user zzzzz-tpzed-xurymjxw79nv3jz') unsatisfied expectations: - expected never, invoked once: #<ActiveSupport::Logger:0x564654464998>.info(any_parameters)
Running in jenkins now: https://ci.curoverse.com/job/developer-run-tests-services-api/328/
#7
Updated by Peter Amstutz almost 5 years ago
Turns out I have a test configuration override of "log_reuse_decisions: true" so false alarm. Please merge.
#8
Updated by Tom Clegg almost 5 years ago
- Status changed from In Progress to Resolved
Merge branch '11803-repo-perms'
refs #11803
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curoverse.com>