Bug 462305

Just tried to reverify that this is still occurring after the performance issues with Bugzilla had been fixed and this still occurs:

Proxy Error

The proxy server received an invalid response from an upstream server.
The proxy server could not handle the request GET /editcomponents.cgi.

Reason: Error reading from remote server

Basically due to the large number of components in Bugzilla for Fedora, and the way editcomponents.cgi loads them from the database, the proxy is timing out before the editcomponents.cgi page is finished loading. We will need to investigate how to speed up the load time of this page and others products with large component numbers as well.

Created attachment 328599 [details]
Patch to speed up loading of editcomponents.cgi for larger comp lists (v1)

Hey Noura. Attaching a patch to speed up the loading of the editcomponents.cgi by hitting the database directly. The way the upstream has it using objects for everything is a large speed decrease with the amount of components we are using for some of our products. Also in admin/components/list.html.tmpl they are creating even more memory use by creating an override data structure for replacing userids with login_names which I am bypassing. It is a significant increase in speed without change in UI.

Let me know what you think.
Dave

Created attachment 328683 [details]
Patch to speed up loading of editcomponents.cgi for larger comp lists (v2)

Needed to add a trick_taint($product_name) before the SQL call.

Dave

Comment on attachment 328683 [details]
Patch to speed up loading of editcomponents.cgi for larger comp lists (v2)

Hey Dave,

Patch looks good, only one thing, it is actually not working properly for components that has no qa_contact i believe this issue is coming from the query

>Index: editcomponents.cgi
>===================================================================

>+
>+    $query .= ", products, profiles devel, profiles qa " .
>+              "WHERE components.product_id = products.id AND products.name = ? " .
>+              "AND components.initialowner = devel.userid " .
>+              "AND components.initialqacontact = qa.userid ";
>+

Not all components have initialqacontact so the condition 
"AND components.initialqacontact = qa.userid " will not be met in that case. Also we need to handle this in the display so we display an empty box for undefined qa_contacts

Thanks,
Noura

Created attachment 328765 [details]
Patch to speed up loading of editcomponents.cgi for larger comp lists (v3)

Thanks for the review Noura. I have updated editcomponents.cgi to account for null qa contacts and works better now.

Please review
Dave

Created attachment 328850 [details]
v4 patch to display loginnames correctly in editcomponents.cgi

Hey Dave,

I did another test for the patch, the previous issue with the null qa_contacts now is fixed, but i found another issue with the display of the login names of the default assignee and qa contact in the editcomponents.cgi, so basically if the page was loaded with no action then it displays fine but if it was loaded after add/delete/update action of a component then the loginnames will display Bugzilla::User hash, that is because default_assignee and default_qa_contact they actually return user object, so one solution i thought of was to add new accessor function to Bugzilla::Component that will return the login name of the qacontact and the assignee of the component and then i fixed your sql accordingly and fixed the name in the template as well this solution seems to work fine, attached is a patch, but i don't know maybe you have better solution. 

Thanks,
Noura

Created attachment 328920 [details]
Patch to speed up loading of editcomponents.cgi for larger comp lists (v4)

Hey Noura. That won't work cause it still tries to load the entire product object with component objects when a component is changed and the components list is redisplayed. So that would be slow reloading as before the fix.

I mistakenly did not take into account reloading of the components list after an edit as I suspected that a redirect was occurring. When actually it is reloading the components/list.html.tmpl directly instead.

I put the SQL that loads the data into a separate function that is now called from each point in editcomponents.cgi that reloads the component list.

Please let me know what you think.
Dave

Comment on attachment 328920 [details]
Patch to speed up loading of editcomponents.cgi for larger comp lists (v4)

Hey Dave,

Patch looks good I tested it and it works fine, but to be able to test it i had to make the variables:

$product_name and $showbugcounts global by replacing 'my' with 'our' in their declaration.

Thanks,
Noura

Created attachment 328943 [details]
Patch to speed up loading of editcomponents.cgi for larger comp lists (v5)

Thanks Noura. I need to start doing the httpd restart thing in the future for my testing as I did not see the error. The error is mod_perl related. I have changed the load_product_data() function to take the $product_name and $showbugcounts variables as arguments which will solve the warning.

Rolling over review from previous patch and committing to CVS.

Thanks
Dave