Arvados

Changes at arvados|c3ba804dfbf425909f465cb26a9395fd488b7b99 branch 17074-optimize-itemsavailable
Tests developer-run-tests-services-workbench2: #1110

• All agreed upon points are implemented / addressed.
  • Changed itemsAvailable in DE to optional, added action/reducer to set itemsAvailable
  • Cleanup and refactor various DE middewares to prepare for changes
  • Changed criteriachanged to default to true
    • criteriachanged was previously only used by the search page and left undefined in other DEs, except when the middleware calls certain reducers (page change, rows per page change). This change ensures criteriachanged is always set to true unless explicitly set to false like in the aforementioned exception cases. I think it makes sense for the default to be true since it will only be false in these special cases like changing pages
    • This allows using criteriachanged to control / prevent re-requesting count when paging vs changing filters
  • Prevent DE from applying updates if DE is pending (and therefore is stale) - reduces appearance of flickering
  • Added loading flag and indicator to pagination component
  • Added requestCount method to DataExplorerMiddlewareService and implement in all DE middlewares
    • A few can't be implemented currently so they just have a stub for now and they continue to request count with the main request
      • search results has special logic involving itemsAvailable
      • user profile groups uses a filtered list query
      • The workflow panel at /workflows is unused and doesn't load anything afaict so I skipped that one
  • Added separate state machine to prevent race conditions when requesting count, follows the REQUEST_ITEMS state machine pattern to handle keeping track of stale count requests and re-issue count requests
    • I decided to break from the pattern slightly and not reissue requests if the request fails to prevent an infinite loop of requests which is possible for the REQUEST_COUNT state machine
  • Updated tests
• Anything not implemented (discovered or discussed during work) has a follow-up story.
  • n/a
• Code is tested and passing, both automated and manual, what manual testing was done is described
  • Updated tests to check for the count request
  • Manually tested to make sure race conditions aren't possible
• Documentation has been updated.
  • n/a
• Behaves appropriately at the intended scale (describe intended scale).
  • Strictly improvements to scale - shared with me on tordo used to take 4.6 seconds, it now takes 0.3s with the count filling in after 3 or so seconds.
• Considered backwards and forwards compatibility issues between client and server.
  • No issues
• Follows our coding standards and GUI style guidelines.
  • yes

Stephen Smith wrote in #note-30:

Changes at arvados|c3ba804dfbf425909f465cb26a9395fd488b7b99 branch 17074-optimize-itemsavailable
Tests developer-run-tests-services-workbench2: #1110

• All agreed upon points are implemented / addressed.
  • Changed itemsAvailable in DE to optional, added action/reducer to set itemsAvailable
  • Cleanup and refactor various DE middewares to prepare for changes
  • Changed criteriachanged to default to true
    • criteriachanged was previously only used by the search page and left undefined in other DEs, except when the middleware calls certain reducers (page change, rows per page change). This change ensures criteriachanged is always set to true unless explicitly set to false like in the aforementioned exception cases. I think it makes sense for the default to be true since it will only be false in these special cases like changing pages
    • This allows using criteriachanged to control / prevent re-requesting count when paging vs changing filters
  • Prevent DE from applying updates if DE is pending (and therefore is stale) - reduces appearance of flickering
  • Added loading flag and indicator to pagination component
  • Added requestCount method to DataExplorerMiddlewareService and implement in all DE middlewares
    • A few can't be implemented currently so they just have a stub for now and they continue to request count with the main request
      • search results has special logic involving itemsAvailable
      • user profile groups uses a filtered list query
      • The workflow panel at /workflows is unused and doesn't load anything afaict so I skipped that one
  • Added separate state machine to prevent race conditions when requesting count, follows the REQUEST_ITEMS state machine pattern to handle keeping track of stale count requests and re-issue count requests
    • I decided to break from the pattern slightly and not reissue requests if the request fails to prevent an infinite loop of requests which is possible for the REQUEST_COUNT state machine
  • Updated tests
• Anything not implemented (discovered or discussed during work) has a follow-up story.
  • n/a
• Code is tested and passing, both automated and manual, what manual testing was done is described
  • Updated tests to check for the count request
  • Manually tested to make sure race conditions aren't possible
• Documentation has been updated.
  • n/a
• Behaves appropriately at the intended scale (describe intended scale).
  • Strictly improvements to scale - shared with me on tordo used to take 4.6 seconds, it now takes 0.3s with the count filling in after 3 or so seconds.
• Considered backwards and forwards compatibility issues between client and server.
  • No issues
• Follows our coding standards and GUI style guidelines.
  • yes

This LGTM!

This is a great improvement! There's still a bit of delay in loading a page, but it doesn't seem to be waiting for the server, so I suspect next thing to focus on will be streamlining browser rendering (and maybe inefficient data handling in the redux store).