Project

General

Profile

Actions

Bug #15672

closed

List of subprocesses limited in process view

Added by Moritz Gilsdorf over 4 years ago. Updated about 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Story points:
2.0

Description

In order to make large numbers of subprocesses readily available and easily navigable to users:

  • Switch to using the DataExplorer WB2 component.
  • Make filtering on state mutually exclusive and use the built in filtering of the DataExplorer. This will require changing the column filter pop up to be radio buttons.
  • Remove the toggles from the subprocess panel, but keep the data about the number of subprocesses in each state.

Files


Subtasks 2 (0 open2 closed)

Arvados - Task #15745: Review 15672-filters-translation-fixResolvedLucas Di Pentima12/16/2019Actions
Arvados - Task #15945: Review 15672-subprocess-list-v2ResolvedPeter Amstutz12/20/2019Actions

Related issues

Related to Arvados - Idea #13327: More responsive page loads with API ?include=container_uuidNewActions
Related to Arvados - Feature #12917: Support ?include=container_uuid for container request lists and group contentsNewTom CleggActions
Related to Arvados - Idea #15350: [Documentation] Add Python SDK recipe on how to get a container request's real stateResolvedLucas Di Pentima06/11/2019Actions
Related to Arvados - Feature #15846: [Workbench2] Data explorer page navigationNewActions
Related to Arvados - Feature #15844: [Workbench2] Indicate when a DataExplorer filter is activeNewActions
Related to Arvados Workbench 2 - Feature #15012: Add /all_processes pageResolvedLucas Di Pentima01/21/2020Actions
Related to Arvados - Bug #15951: Process filtering UI polishResolvedLucas Di Pentima03/04/2020Actions
Related to Arvados - Bug #15902: Very slow query for large workflowsResolvedTom Clegg01/28/2020Actions
Blocked by Arvados - Feature #15854: Add container request filtering based on container.exit_code and container.stateResolvedTom Clegg11/20/2019Actions
Actions #1

Updated by Moritz Gilsdorf over 4 years ago

There seems to be a limit in how many sub-processes are listed in the process view. In the workflow run shown in the attached screenshot are in total 11169 steps but the api seems to only fetch the first 1000.

!Screenshot 2019-09-30 at 10.44.03.png!

Actions #2

Updated by Tom Morris over 4 years ago

  • Priority changed from Normal to High
Actions #3

Updated by Tom Morris over 4 years ago

  • Target version changed from Workbench2 Q3, Q4 to 2019-11-06 Sprint
Actions #4

Updated by Eric Biagiotti over 4 years ago

  • Assigned To set to Eric Biagiotti
Actions #5

Updated by Peter Amstutz over 4 years ago

  • Related to Idea #13327: More responsive page loads with API ?include=container_uuid added
Actions #6

Updated by Tom Morris over 4 years ago

  • Story points set to 2.0
Actions #7

Updated by Eric Biagiotti over 4 years ago

  • For listings, use posts with _method=GET so we won't get a 414 and load them into state.
  • Test performance with 11k+ entries.
  • If necessary add virtualization to the grid.
Actions #8

Updated by Eric Biagiotti over 4 years ago

  • Status changed from New to In Progress
Actions #9

Updated by Eric Biagiotti over 4 years ago

  • Target version changed from 2019-11-06 Sprint to 2019-11-20 Sprint
Actions #10

Updated by Eric Biagiotti over 4 years ago

I am currently working on adding a data explorer to the process/subprocess view (the notion of a process is meant to simplify the container request and container relationship in WB2). The data explorer works by using the list API to only get the data that it needs (# of rows being displayed). Normally, it is trivial to filter because you just update the list query.

But, filtering on process status is presenting a problem because it is based on the container and the container request state attribute (e.g. a process is cancelled if the container request priority is 0, or if the container state is "Cancelled").

Example: I want to display 10 rows or subprocesses excluding ones that are cancelled.

  • List and filter container_requests where priority != 0, which returns 10 results.
  • Use the containerUuid of the returned container_request objects to get the container (if it exists) and filter where state != "Cancelled". Lets say this returns 9 results because one container was cancelled.
  • In this state, we don't have enough data to fill all the rows.

The only solution I can think of is to first get all the subprocesses (container requests where requesting_container_uuid is the parent container uuid and the corresponding containers if they exists). Then, I can filter them in the code. But depending on the number of subprocesses and the API request limit, this could add many more network requests and kind of defeats the purpose of the data explorer.

Or maybe I am missing something? Let me know if any of this makes sense. Thanks!

Actions #11

Updated by Eric Biagiotti over 4 years ago

A few potential approaches to consider:

Assumptions: All crs have containers. Status filtering operates only on the state/exit_code of the container. Data explorer implementations will have search bars using the trigram index.

Filtering with data explorer
  • Approach: Get the containerUuid of all the sub-crs that are COMITTED or FINAL (has a container). This requires adding client functionality to bypass API server response limits. Use the list method to get corresponding containers and filter on the containerUuids and "state"/"exit_code" and limit on number of rows to display.
  • Pros: Keeps current filtering functionality and adds a data explorer, presenting more useful info to the user.
  • Cons: Large network requests, scalability problems.
WB1 style with no filtering and infinite scroll
  • Approach: Remove filtering on status and put the data explorer in infinite scroll mode. This adds a button at the bottom of the page that says "Load More" and is more like how WB1 works. This could still result in rendering performance issues unless we block this behind the react-redux update and try using list virtualization.
  • Pros: Network friendly and more scalable.
  • Cons: Removes functionality from the process page. Could still result in rendering issues with a full list displayed.

New API feature

  • Approach: Block this behind adding an API feature to be able to filter container requests by their container's state and exit_code.
  • Pros: Network friendly and more scalable. Data explorer will work well and no filtering needs to be done on the client side. Users keep existing filters.
  • Cons: API work has to be done first.
Actions #12

Updated by Peter Amstutz over 4 years ago

Eric Biagiotti wrote:

A few potential approaches to consider:

Assumptions: All crs have containers. Status filtering operates only on the state/exit_code of the container. Data explorer implementations will have search bars using the trigram index.

Filtering with data explorer
  • Approach: Get the containerUuid of all the sub-crs that are COMITTED or FINAL (has a container). This requires adding client functionality to bypass API server response limits. Use the list method to get corresponding containers and filter on the containerUuids and "state"/"exit_code" and limit on number of rows to display.
  • Pros: Keeps current filtering functionality and adds a data explorer, presenting more useful info to the user.
  • Cons: Large network requests, scalability problems.
WB1 style with no filtering and infinite scroll
  • Approach: Remove filtering on status and put the data explorer in infinite scroll mode. This adds a button at the bottom of the page that says "Load More" and is more like how WB1 works. This could still result in rendering performance issues unless we block this behind the react-redux update and try using list virtualization.
  • Pros: Network friendly and more scalable.
  • Cons: Removes functionality from the process page. Could still result in rendering issues with a full list displayed.

New API feature

  • Approach: Block this behind adding an API feature to be able to filter container requests by their container's state and exit_code.
  • Pros: Network friendly and more scalable. Data explorer will work well and no filtering needs to be done on the client side. Users keep existing filters.
  • Cons: API work has to be done first.

Maybe we should have a quick meeting on this next week but it feels to me like doing it all client side is actually a nontrivial amount of work for an inferior result, whereas if we add an API side query that does exactly what we want (which I believe would be relatively easy), we will also be able to plug it into the existing data table with minimal effort.

Actions #13

Updated by Tom Morris over 4 years ago

  • Related to Feature #12917: Support ?include=container_uuid for container request lists and group contents added
Actions #14

Updated by Tom Morris over 4 years ago

  • Related to Idea #15350: [Documentation] Add Python SDK recipe on how to get a container request's real state added
Actions #15

Updated by Eric Biagiotti over 4 years ago

  • Description updated (diff)
Actions #16

Updated by Eric Biagiotti over 4 years ago

  • Target version changed from 2019-11-20 Sprint to 2019-12-04 Sprint
Actions #17

Updated by Eric Biagiotti over 4 years ago

  • Description updated (diff)
Actions #18

Updated by Eric Biagiotti over 4 years ago

  • Blocked by Feature #15854: Add container request filtering based on container.exit_code and container.state added
Actions #19

Updated by Eric Biagiotti over 4 years ago

  • Related to Feature #15846: [Workbench2] Data explorer page navigation added
Actions #20

Updated by Eric Biagiotti over 4 years ago

  • Related to Feature #15844: [Workbench2] Indicate when a DataExplorer filter is active added
Actions #21

Updated by Eric Biagiotti over 4 years ago

  • Target version changed from 2019-12-04 Sprint to 2020-01-02 Sprint
Actions #22

Updated by Eric Biagiotti over 4 years ago

- Docs updated @ commit:d953b5510676f1cb5cfab7a092d2de2cd3c72135 in the Arvados repo.

Latest work:

- Added the ability to create radio button groups for data explorer column filtering.
- Process panel now stores the parent container_request uuid, which is used as a prop to the subprocess panel to populate the dataexplorer.
- Added Run Time and Created At columns. Can't currently sort by runtime.

TODO:

- Add started_at and finished_at time to columns. Users can use the column filtering on the top right of the data explorer to choose what columns to display. Pick a reasonable set of defaults.
- The action triple dot for subprocesses in the data explorer is not set up. Look at project-panel.tsx for an example.
- Search of the subprocess view from the data explorer doesn't work. Use trigram index API call for this.
- Default message on the data explorer for when there are no subprocesses in the view is not being displayed.
- Clicking on a subprocess in the data explorer should update the details panel.
- Navigation when double clicking on a subprocess isn't working.
- Reset the data explorer radio box when navigating to the page?

Actions #23

Updated by Lucas Di Pentima over 4 years ago

  • Assigned To changed from Eric Biagiotti to Lucas Di Pentima
Actions #24

Updated by Peter Amstutz over 4 years ago

Actions #25

Updated by Lucas Di Pentima over 4 years ago

Filters were being constructed by doing snake case translation on the attribute name.
Branch 15672-filters-translation-fix at commit 680b1ffd (wb2 repo) removed this translation and fixes all parts of the code using the FilterBuilder class.

This is needed on this branch because the filter [['container.state', '=', 'foo']] gets translated to [['container_state', '=', 'foo']].

Also found a similar case on ordering, but that is being type checked against the resources' key names so I think it's not so trivial to fix.

Actions #26

Updated by Peter Amstutz over 4 years ago

Lucas Di Pentima wrote:

Filters were being constructed by doing snake case translation on the attribute name.
Branch 15672-filters-translation-fix at commit 680b1ffd (wb2 repo) removed this translation and fixes all parts of the code using the FilterBuilder class.

This is needed on this branch because the filter [['container.state', '=', 'foo']] gets translated to [['container_state', '=', 'foo']].

Also found a similar case on ordering, but that is being type checked against the resources' key names so I think it's not so trivial to fix.

This LGTM.

Actions #27

Updated by Lucas Di Pentima over 4 years ago

Updates at commit 134e0d36 - branch 15672-subprocess-list-v2 (wb2 repo)

Continued Eric's branch on a separate one, having rebased it to use the previous fixes on filter translation:

  • Made status filters added by Eric to be used on the request
  • Pagination options now offer 50, 100, 200 and 500 page sizes
  • Removed subprocess count and filter sliders panel on the top right corner
  • Double click and name-click navigate to the selected subprocess
  • Single click updates the detail panel
  • When no subprocesses available to list (because of filtering, search, etc) show a message.
  • Modified the list api calls to use POST only when the params on the query string are big enough (>1500 chars)
  • Added subprocess search by name, not sure if we should be able to search by other properties
Actions #28

Updated by Peter Amstutz about 4 years ago

Functional comments, haven't looked at the code yet:

Locked and running containers have a runtime of "NaNh NaNm NaNs" (running containers should compute it as now - started_at, Locked should be either blank or zero)

In the "Status filters" popup, clicking on the button works to change the filter. However moving the mouse cursor over the text of an item and clicking it changes the item background but doesn't change the filter. Also it seems unnecessary to select the filter and then separately choose Ok/Cancel, a single click should select the item and close the dialog.

After closing the "Status filters" popup, the "Filters" tooltip remains on the screen, partly obscuring the first row.

The list live-updates the state of individual subprocess items but does not refresh the list. So if you filter on "Running" the items in the list can live-update to "Completed" but are not removed from the list, and new "Running" subprocesses are not added. There's also no button to explicitly refresh the list (this something that is missing from the UI design in general, clicking the item in the breadcrumb bar usually works but is not at all obvious).

Actions #29

Updated by Peter Amstutz about 4 years ago

Code @ 16860c6f2fedc5c150cab796e93aa21067f28e97

subprocess-panel-actions.ts has a bunch of commented-out code

I see there's a ContainerRunTime component so it shouldn't be too hard to fix the time display (although since 'now' constantly changes, I'm not sure about the preferred way to handle that sort of thing in React, we probably only want update granularity to the nearest minute).

Actions #30

Updated by Peter Amstutz about 4 years ago

Switching from GET to POST when the URI gets too long -- could we unit test that?

Actions #31

Updated by Peter Amstutz about 4 years ago

  • Related to Bug #15951: Process filtering UI polish added
Actions #32

Updated by Peter Amstutz about 4 years ago

  • Target version changed from 2020-01-02 Sprint to 2020-01-15 Sprint
Actions #33

Updated by Peter Amstutz about 4 years ago

  • Assigned To changed from Lucas Di Pentima to Peter Amstutz
Actions #34

Updated by Peter Amstutz about 4 years ago

  • Target version changed from 2020-01-15 Sprint to 2020-01-29 Sprint
Actions #35

Updated by Peter Amstutz about 4 years ago

  • Assigned To changed from Peter Amstutz to Lucas Di Pentima
Actions #36

Updated by Tom Clegg about 4 years ago

  • Related to Bug #15902: Very slow query for large workflows added
Actions #37

Updated by Lucas Di Pentima about 4 years ago

Updates at commit 6183e02

  • Reviewed Peter's additions and fixed a broken test
  • Added a unit test for the list API call with POST method when using big query params as requested on note-30
Actions #38

Updated by Lucas Di Pentima about 4 years ago

Simplified test code @ commit 22a3f545

Actions #39

Updated by Peter Amstutz about 4 years ago

Lucas Di Pentima wrote:

Simplified test code @ commit 22a3f545

My only comment would be to recommend having two test cases, one just under the limit (uses GET) and one just over the limit (uses POST), currently only tests the second case one so every query could use POST and this wouldn't detect it.

Rest LGTM, please merge.

Actions #40

Updated by Lucas Di Pentima about 4 years ago

Added test @ commit ea35db0f

Actions #41

Updated by Anonymous about 4 years ago

  • % Done changed from 50 to 100
  • Status changed from In Progress to Resolved
Actions #42

Updated by Peter Amstutz about 3 years ago

  • Related to Idea #16945: WB2 Workflows / containers feature parity added
Actions #43

Updated by Peter Amstutz about 3 years ago

  • Related to deleted (Idea #16945: WB2 Workflows / containers feature parity)
Actions

Also available in: Atom PDF