Bug 1640908

Summary: Javascript Error popup when Managing StorageDomain with LUNs and 400+ paths
Product: Red Hat Enterprise Virtualization Manager Reporter: Steffen Froemer <sfroemer>
Component: ovirt-engineAssignee: rszwajko
Status: CLOSED ERRATA QA Contact: mlehrer
Severity: high Docs Contact:
Priority: unspecified    
Version: 4.2.0CC: dagur, gshereme, guchen, gveitmic, lleistne, michal.skrivanek, rdlugyhe, rszwajko, sgratch, sraje, tashkena, tnisan
Target Milestone: ovirt-4.4.1Keywords: Performance, Rebase
Target Release: ---Flags: lsvaty: testing_plan_complete-
Hardware: x86_64   
OS: All   
Whiteboard:
Fixed In Version: rhv-4.4.1-3 Doc Type: Enhancement
Doc Text:
Previously, if there were hundreds of Fiber Channel LUNs, the Administration Portal dialog box for adding or managing storage domains took too long to render and might become unresponsive. This enhancement improves performance: It displays a portion of the LUNs in a table and provides right and left arrows that users can click to see the next or previous set of LUNs. As a result, the window renders normally and remains responsive regardless of how many LUNs are present.
Story Points: ---
Clone Of: Environment:
Last Closed: 2020-08-04 13:16:11 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: UX RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
performance on Chromium with static data (500 LUNs)
none
performance on Firefox with static data (500 LUNs)
none
offline paging added above the table
none
paging - perfomrance with 500 mocked LUNs on Chromium none

Description Steffen Froemer 2018-10-19 07:00:22 UTC
Description of problem:
When trying to manage or create a StorageDomain, where LUNs attached with 400+ paths, you will get an javascript error

Version-Release number of selected component (if applicable):
ovirt-engine-4.2.6.4-0.1.el7ev.noarch

How reproducible:
always

Steps to Reproduce:
1. connect enough FC-devices to hosts until you have 400+ paths 
2. Open New StorageDomain dialog / or modify an existing one
3. Select StorageType 'FibreChannel'

Actual results:
The dialog stuck and will show Javascript Error ("not responding script") after a while

Expected results:
A domain should be 

Additional info:
The dialog was similar slow in RHV-4.1 UI, but on patternfly it might be worse

Comment 3 Tal Nisan 2018-10-22 08:33:37 UTC
Greg, from the storage perspective the code works as it should and creates the dialog, I guess the massive amount of elements "chokes" the browser, what can we do to fix it?

Comment 4 Greg Sheremeta 2018-10-24 00:45:02 UTC
profile it. you can see if there's some method the app is spending a ton of time in. Or you can see if there's a memory leak in that component.

I can assist if someone has an environment with FC luns (or if there's a way to stub them in via database edits)

Comment 5 Tal Nisan 2018-10-28 15:06:44 UTC
Guy, can you provide Greg with a host with 400+ paths connected?

Comment 6 Greg Sheremeta 2018-10-30 17:37:38 UTC
Shani got me what I needed -- thanks!

Comment 7 Tal Nisan 2018-11-05 09:17:47 UTC
OK, so moving to UX now to note that you are currently handling it

Comment 11 Daniel Gur 2018-12-30 09:35:54 UTC
No corrent need from QE (Guy)

Comment 12 Sandro Bonazzola 2019-01-28 09:41:20 UTC
This bug has not been marked as blocker for oVirt 4.3.0.
Since we are releasing it tomorrow, January 29th, this bug has been re-targeted to 4.3.1.

Comment 14 Michal Skrivanek 2019-03-18 08:58:39 UTC
Greg, any updates?

Comment 15 Greg Sheremeta 2019-03-19 14:27:32 UTC
(In reply to Michal Skrivanek from comment #14)
> Greg, any updates?

No.

Comment 17 Daniel Gur 2019-08-28 13:14:30 UTC
sync2jira

Comment 18 Daniel Gur 2019-08-28 13:19:32 UTC
sync2jira

Comment 19 Michal Skrivanek 2020-03-18 15:45:41 UTC
This bug didn't get any attention for a while, we didn't have the capacity to make any progress. If you deeply care about it or want to work on it please assign/target accordingly

Comment 20 Michal Skrivanek 2020-03-18 15:50:37 UTC
This bug didn't get any attention for a while, we didn't have the capacity to make any progress. If you deeply care about it or want to work on it please assign/target accordingly

Comment 22 rszwajko 2020-04-03 11:31:57 UTC
The problem can be reproduced by replacing the response with static data (SanStorageModelBase.updateInternal()):

-                model.applyData((ArrayList<LUNs>) response.getReturnValue(), false, prevSelected,
+                List<LUNs> stubs = stub(500);
+                model.applyData(stubs, false, prevSelected,

Perfomance tests results (on my laptop):
1. Chromium ~50 sec (see attachment performance_500_mocked_LUNs_chromium.webm)
2. Firefox ~22sec (see performance_500_mocked_LUNs_firefox.webm)

Comment 23 rszwajko 2020-04-03 11:33:25 UTC
Created attachment 1675993 [details]
performance on Chromium with static data (500 LUNs)

Comment 24 rszwajko 2020-04-03 11:36:11 UTC
Created attachment 1675994 [details]
performance on Firefox with static data (500 LUNs)

Comment 25 rszwajko 2020-04-09 12:54:31 UTC
Created attachment 1677534 [details]
offline paging added above the table

Comment 26 rszwajko 2020-04-09 14:51:21 UTC
Profiling shows that rendering stage is the most time consuming. 
The LUNs table at Domains -> New Domain -> Fiber Channel is technically speaking a tree in which each node (row) is a GWT table component.
Scenario described in this issue does not require all of the functionality (i.e. capability of displaying another GWT table as child node).
However since the component is quite complex it would be time consuming to build a special purpose component only for this scenario.
Due to this fact paging approach has been chosen.

Solution is based on the paging code that was used so far in all tables (bottom right corner above the table).
It has been extracted to allow re-use in the described scenario - see comment #25 for screen shot with the new look&feel.
Note that for Fiber Channel screen the total number of items is displayed - differently then on other screens.
This is caused by the fact that the underlying API does not support server side paging so all items are loaded in one request.
Since the data is stored in the browser we know the size of the result set and are able to show this to the user.
It's a small step towards design recommended by PatternFly 3 & 4 (see below).

https://www.patternfly.org/v3/pattern-library/navigation/pagination/index.html
https://www.patternfly.org/v4/design-guidelines/usage-and-behavior/pagination#main-content

Comment 27 rszwajko 2020-04-09 15:32:22 UTC
Created attachment 1677572 [details]
paging - perfomrance with 500 mocked LUNs on Chromium

Comment 31 Sharon Gratch 2020-05-13 06:34:12 UTC
Since it is MODIFIED already then re-targeting to 4.4.1

Comment 35 Tzahi Ashkenazi 2020-06-22 14:24:23 UTC
Verified  on :
      rhv-release-4.4.1-3-001.noarch
      vdsm-common-4.40.19-1.el8ev.noarch

created 300 luns with 4 paths total  1200  FC 


from the UI : 

Storage > Doamin > New > provide the current  DC / Cluster / FC / Host 

all the Luns available on  the UI  300 luns on 6 pages each page 50 luns

Comment 44 errata-xmlrpc 2020-08-04 13:16:11 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Important: RHV Manager (ovirt-engine) 4.4 security, bug fix, and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2020:3247