Bug #8183

[Workbench] should not look up every group/project a user has access to on every page load

Added by Ward Vandewege over 3 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
Workbench
Target version:
Start date:
02/12/2016
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
1.0

Description

A user created over 20,000 groups on one of our installations.

For admin users (who can see every group) and for the user in question, Workbench appears to request all of these groups, 100 at a time, for each page load. An example from the api server logs:

...
10.28.0.7 - - [11/Jan/2016:19:33:58 +0000] "POST /arvados/v1/groups HTTP/1.1" 200 53511 "-" "HTTPClient/1.0 (2.6.0.1, ruby 2.1.4 (2014-10-27))" 
10.28.0.7 - - [11/Jan/2016:19:33:59 +0000] "POST /arvados/v1/groups HTTP/1.1" 200 52721 "-" "HTTPClient/1.0 (2.6.0.1, ruby 2.1.4 (2014-10-27))" 
10.28.0.7 - - [11/Jan/2016:19:33:59 +0000] "POST /arvados/v1/groups HTTP/1.1" 200 53510 "-" "HTTPClient/1.0 (2.6.0.1, ruby 2.1.4 (2014-10-27))" 
10.28.0.7 - - [11/Jan/2016:19:34:00 +0000] "POST /arvados/v1/groups HTTP/1.1" 200 53510 "-" "HTTPClient/1.0 (2.6.0.1, ruby 2.1.4 (2014-10-27))" 
10.28.0.7 - - [11/Jan/2016:19:34:00 +0000] "POST /arvados/v1/groups HTTP/1.1" 200 53508 "-" "HTTPClient/1.0 (2.6.0.1, ruby 2.1.4 (2014-10-27))" 
10.28.0.7 - - [11/Jan/2016:19:34:00 +0000] "POST /arvados/v1/groups HTTP/1.1" 200 53507 "-" "HTTPClient/1.0 (2.6.0.1, ruby 2.1.4 (2014-10-27))" 
10.28.0.7 - - [11/Jan/2016:19:34:00 +0000] "POST /arvados/v1/groups HTTP/1.1" 200 53512 "-" "HTTPClient/1.0 (2.6.0.1, ruby 2.1.4 (2014-10-27))" 
10.28.0.7 - - [11/Jan/2016:19:34:01 +0000] "POST /arvados/v1/groups HTTP/1.1" 200 53511 "-" "HTTPClient/1.0 (2.6.0.1, ruby 2.1.4 (2014-10-27))" 
10.28.0.7 - - [11/Jan/2016:19:34:01 +0000] "POST /arvados/v1/groups HTTP/1.1" 200 53506 "-" "HTTPClient/1.0 (2.6.0.1, ruby 2.1.4 (2014-10-27))" 
...

This is so slow that it causes the nginx reverse proxy to time out, making workbench unusable. The API server is fine - the cli interface works.

Proposed fix

When building the "my projects" tree for the workbench project dropdowns:
  • fetch one page of "all readable projects" (like we do now, but with "get all pages" turned off)
  • compare the returned result count to items_available
  • if items_available is > 3x returned result count (i.e., it would take more than 2 additional API calls to retrieve the full list), switch tactics and build the "my projects" tree this way instead:
    • set my_project_uuids=[current_user.uuid]
    • retrieve all projects with owner_uuid in my_project_uuids (limit=200)
    • add results to my_project_uuids
    • repeat until no new results arrive, or my_project_uuids has more than 200 entries

For now, the "choose project" dialog should use the old method, even though it will be slow when there are thousands of projects. We will come back and fix this part after #8286.


Subtasks

Task #8364: Review branch: 8183-projects-dropdownResolvedPeter Amstutz


Related issues

Related to Arvados - Bug #8189: [FUSE] Listing a project directory is slow when there are many subprojectsResolved01/11/2016

Related to Arvados - Story #8286: [API] [Workbench] Allow users to choose which projects appear in their dropdown areaResolved01/22/2016

Associated revisions

Revision dafd66c2
Added by Radhika Chippada over 3 years ago

closes #8183
Merge branch '8183-projects-dropdown'

History

#1 Updated by Ward Vandewege over 3 years ago

  • Description updated (diff)

#2 Updated by Ward Vandewege over 3 years ago

  • Description updated (diff)

#3 Updated by Ward Vandewege over 3 years ago

I worked around the problem with this crude patch, which rips out the listing from the Projects dropdown:

workbench:/var/www/workbench.su92l.arvadosapi.com/current/app/views/layouts# diff body.html.erb.bu body.html.erb -uw
--- body.html.erb.bu    2016-01-11 20:25:33.411584442 +0000
+++ body.html.erb    2016-01-11 20:25:39.667584446 +0000
@@ -226,14 +226,6 @@
                 <% end %>
               </li>
               <li role="presentation" class="divider"></li>
-              <%= render partial: "projects_tree_menu", locals: {
-                  :project_link_to => Proc.new do |pnode, &block|
-                    link_to(project_path(pnode[:object].uuid),
-                      data: { 'object-uuid' => pnode[:object].uuid,
-                              'name' => 'name' },
-                      &block)
-                  end,
-              } %>
             </ul>
           </li>
           <% if @name_link or @object %>

Then I also added this patch at Tom's recommendation:

--- app/models/arvados_resource_list.rb.bu    2016-01-11 21:33:11.952660291 +0000
+++ app/models/arvados_resource_list.rb    2016-01-11 20:31:26.727584653 +0000
@@ -184,6 +184,7 @@
     api_params[:order] = @orderby_spec if @orderby_spec
     api_params[:filters] = @filters if @filters
     api_params[:distinct] = @distinct if @distinct
+    api_params[:limit] = 10000

     item_count = 0
     offset = @offset || 0

Note that the API server default max is 1000, so in reality the number of elements returned will be capped at 1000. Still, that's 10x fewer calls than before.

#4 Updated by Ward Vandewege over 3 years ago

  • Subject changed from Workbench should not look up every group a user has access to on every page load to [Workbench] should not look up every group a user has access to on every page load

#5 Updated by Brett Smith over 3 years ago

  • Target version set to Arvados Future Sprints

#6 Updated by Brett Smith over 3 years ago

  • Status changed from New to Closed
  • Story points set to 0.5

#7 Updated by Brett Smith over 3 years ago

  • Status changed from Closed to New

#8 Updated by Tom Clegg over 3 years ago

  • Subject changed from [Workbench] should not look up every group a user has access to on every page load to [Workbench] should not look up every group/project a user has access to on every page load

#9 Updated by Tom Clegg over 3 years ago

  • Description updated (diff)

#10 Updated by Tom Clegg over 3 years ago

  • Target version changed from Arvados Future Sprints to 2016-02-17 Sprint

#11 Updated by Radhika Chippada over 3 years ago

  • Category set to Workbench
  • Assigned To set to Radhika Chippada
  • Story points changed from 0.5 to 1.0

#12 Updated by Radhika Chippada over 3 years ago

The code that builds the project tree is also used by the "move object to project / assign project to object" popup. So, we need to discuss and make sure we address (1) projects dropdown, and (2) project selection popup.

#13 Updated by Tom Clegg over 3 years ago

The trouble is that building a tree requires getting the whole list with tens of thousands of projects. We need to find a way that doesn't build a full tree up front. This leaves us with three ways we can support finding a project:
  1. Start at Home and navigate through that tree (expanding one level means ~1 API call)
  2. Starred projects
  3. Search

How about a tree view that can expand/collapse one item at a time by clicking a caret? That would let you navigate your whole tree without any expensive queries.

(initial)     (after clicking   (after clicking
               ▶Project 2)       ▶Project 2c)

Home          Home              Home
 ▶Project 1    ▶Project 1        ▶Project 1
 ▶Project 2    ▼Project 2        ▼Project 2
 ▶Project 3      ▶Project 2a       ▶Project 2a
 ▶Project 4      ▶Project 2b       ▶Project 2b
                 ▶Project 2c       ▼Project 2c
                                     ▶Project 2c1
                                     ▶Project 2c2

Can we hook up all the AJAX to make that happen in the relevant dropdowns?

I'm guessing it will improve usability if we move this nav element out of dropdowns... hopefully that doesn't have too much impact on the implementation and it can be a separate story.

#14 Updated by Radhika Chippada over 3 years ago

The proposal in note 13 still does not address the entire problem. The tree building code is also used by the popup to move an object into a project. So, assuming we can pull of an expanding tree as in note 13 #8183-13 for "My projects", how do we handle the move object scenario; more specifically the "Projects shared with me" portion of the tree in this popup?

#15 Updated by Radhika Chippada over 3 years ago

branch 8183-projects-dropdown: 97e018a1a8b9d4da6c99a9eb7be36a7f368615f9

  • Projects dropdown is updated to show only toplevel projects. Updated views/application/_projects_tree_menu.html.erb accordingly.
    • Now 'My projects' lists only toplevel projects. One issue I noticed with this: let's say I have a project with 2 or more levels of subprojects. Home -> Project 1 -> Project 1.1 -> Project 1.1.1. The search dialog (accessed by clicking on search link in topnav) shows toplevel projects in 'My projects'. I see Porject 1 in it. If I click on it, the left portion of search dialog now display Project 1.1. I do not have a way of going to Project 1.1.1. The only way to go to it is by knowing it's name and entering it in the search box in the dialog.
    • Being able to build the expandable list as in note-13 #8183-13 should help resolve this issue.
  • The dialog displayed when an object is moved to a project 'views/projects/_choose.html.erb' is untouched and hence will continue to use the previous implementation of tree building

#16 Updated by Radhika Chippada over 3 years ago

  • Status changed from New to In Progress

#17 Updated by Tom Clegg over 3 years ago

  • Description updated (diff)

#18 Updated by Peter Amstutz over 3 years ago

The message "Showing X of your projects out of Y total projects" gives a count of all projects that accessible to you. However, this is misleading because the count includes projects shared with you that are currently never displayed in the menu. If possible, fix it to give the number projects you own, otherwise I suggest removing one or both of the numbers and changing the text to say something like "Showing your top projects, some projects have been omitted".

#19 Updated by Peter Amstutz over 3 years ago

On further inspection, the behavior is even more confusing for the user:

If there are less than page_size*3 projects, they can see all projects (including those shared).

But if there are more than page_size*3 projects, then the shared projects go away entirely.

Also, if there are more than page_size*3 projects, the first request assigned to "all" is not used which means it the time spent making the request was wasted.

I suggest the routine should only use the code path that builds the tree for the user's own projects:

  def my_wanted_projects page_size=100
    return @my_wanted_projects if @my_wanted_projects

    from_top = []
    uuids = [current_user.uuid]
    while from_top.size <= page_size*2
      current_level = Group.filter([['group_class','=','project'],
                                    ['owner_uuid', 'in', uuids]])
                      .order('name').limit(page_size*2)
      break if current_level.results.size == 0
      from_top.concat current_level.results
      uuids = current_level.results.collect { |x| x.uuid }
    end
    @my_wanted_projects = from_top
  end

#21 Updated by Radhika Chippada over 3 years ago

Implemented the update suggested in note 19 at 42db5188e104e94ce73d743edaafb1c2053e3c0c

Regarding note 20: We currently (already) do not show shared project. And, from conversations with Tom about this for this ticket and #8286, Tom wants to remove it from the chooser dialog also and instead wants the user to use the favorite projects feature to have access to them in this context.

I will check with Tom on Monday and act accordingly. Thanks.

#22 Updated by Peter Amstutz over 3 years ago

We can probably put aside the question of whether we want to show shared projects for now.

However, I have another suggestion, which to limit the loop in #19 to up to three iterations, so it only shows projects up to three levels deep (and only makes a maximum of 3 API calls.)

#23 Updated by Radhika Chippada over 3 years ago

884558f4505049685c7cdb5b50c4b8948f38cd1b

Enhanced code to display only the top three levels of projects. Improved the message displayed when projects / levels are omitted. Improved test a little more.

#24 Updated by Radhika Chippada over 3 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:dafd66c2a336939739ee773b5dd3c65b69042fbb.

Also available in: Atom PDF