Bug 1037616

Summary: Pull calls to find current availability out of the processing loop
Product: [Other] RHQ Project Reporter: Heiko W. Rupp <hrupp>
Component: Core Server, PerformanceAssignee: Jay Shaughnessy <jshaughn>
Status: CLOSED CURRENTRELEASE QA Contact: Mike Foley <mfoley>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 4.9CC: hrupp, jshaughn
Target Milestone: ---   
Target Release: RHQ 4.10   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-04-23 12:31:53 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Patch described none

Description Heiko W. Rupp 2013-12-03 13:36:25 UTC
Created attachment 832060 [details]
Patch described

We are doing in Availmanager.mergeAvailInNewTx

for ( Avail a: incoming ) {

   latest = db.find ( a );

   ...

   latestRA = db.find(ResourceAvail.class, a.resoure.id);
}

So with a report of 10k availabilities we fire >20k db requests and round trips to the database to just get the current state of affairs.
We should pull that out to reduce the number of round trips to the DB.

See patch that illustrates what I mean (we should use probably use named queries, that the JPA subsystem can pre-compile into prepared statements).

Comment 1 Heiko W. Rupp 2013-12-03 13:38:15 UTC
See also Bug 1032192 that increases batch sizes for writing the (changed) data back to the DB.

Comment 2 Jay Shaughnessy 2013-12-03 22:28:27 UTC
I agree that we could avoid some db fetches here.  I'm not sure how significant a boost we'll see but this is a critical code section for scalability and perhaps any boost is worth a change.  I've reviewed the patch and will apply similar changes. Although I don't think the patch as it stands is quite right in its approach to the previous exception handling cases.

Comment 3 Jay Shaughnessy 2013-12-05 16:40:32 UTC
master commit 544dc80ce4c22e399d3ca607108896feb6ebd5b6
Author: Jay Shaughnessy <jshaughn>
Date:   Wed Dec 4 13:36:49 2013 -0500

    Optimization work based on BZ-supplied patch. For a batch of Availabilities
    being processed, pull latest availabilities for the resources  in one DB
    round trip as opposed to once for each resource.  Still beiong careful to
    repair issues when no, or multiple, latest entries are found.

master commit 0beeff7ee267456050c572124b5e4103bd377c1c
Author: Jay Shaughnessy <jshaughn>
Date:   Thu Dec 5 11:37:30 2013 -0500

    Part 2 - batch the pull of ResourceAvailability records that need to
    be updated due to changed availability. Pull only the necessary records.



Test Notes:
This is a performance optimization.  Testing here is implicit, given no regressions in existing testing, this can be marked verified.

Comment 4 Jay Shaughnessy 2013-12-06 21:05:04 UTC
master commit b84dcfa9e3f141d2431665b592cab9c95655a0e7
Author: Jay Shaughnessy <jshaughn>
Date:   Fri Dec 6 16:01:03 2013 -0500

    A fix in the new impl to handle multiple  updates for the same resource in
    the same report.  In general an avail report should only have one update for
    one resource, but the potential for building up reports looms in the
    future, and we do have this scenario in test code.

Comment 5 Heiko W. Rupp 2014-04-23 12:31:53 UTC
Bulk closing of 4.10 issues.

If an issue is not solved for you, please open a new BZ (or clone the existing one) with a version designator of 4.10.