Bug 1426745

Summary: beakerd wastes time counting rows before fetching them
Product: [Community] Beaker Reporter: Dan Callaghan <dcallagh>
Component: generalAssignee: Dan Callaghan <dcallagh>
Status: CLOSED CURRENTRELEASE QA Contact: tools-bugs <tools-bugs>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 24CC: dcallagh, mjia, rjoost
Target Milestone: 24.2Keywords: Patch
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-03-30 03:23:03 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Dan Callaghan 2017-02-24 18:05:50 UTC
On a heavily loaded Beaker instance, like our production one, beakerd's scheduling routines can become a bottleneck. In particular the scheduler does a lot of work on the job and system tables, and is contending for locks with HTTP requests for in-progress jobs (recording results, etc).

To help reduce latency and eliminate lock contention as much as possible, beakerd should avoid doing any unnecessary queries. But currently each scheduling loop is structured in the form:

* count if there are rows that need attention
* if there is,
   * log a message to say I'm doing work
   * loop over the rows that need attention (this is a separate query)
       * do the work
   * log a message to say I'm done

We can eliminate the first count in favour of just iterating over the actual rows. There will be zero iterations of the loop if there is nothing to do.

One neat thing about the current approach is that it means the scheduler does not log anything if it is not doing any work, which means on a quiescent Beaker system the beakerd.log file stays quiet instead of filling with pointless spew. However I think we can just tweak the logging (specifically, ensure we log *inside* each loop iteration but not outside) to achieve roughly the same effect, while still avoiding the unnecessary COUNT() queries.

Comment 1 Dan Callaghan 2017-02-24 21:29:04 UTC
https://gerrit.beaker-project.org/5651

Comment 2 Roman Joost 2017-03-06 23:56:52 UTC
We'll have to do QE on the items before we can tag a release.

Comment 3 Roman Joost 2017-03-20 07:32:01 UTC
Tested on my own beaker installation (Beaker 24.2.git.9.d4c983d). Scheduled a few jobs and with reservations:

Mar 20 07:38:13 beaker-server-lc beakerd[32477]: bkr.server.tools.beakerd DEBUG Created watchdog for recipe id: 6 and system: beaker-test-vm2
Mar 20 07:38:13 beaker-server-lc beakerd[32477]: bkr.server.tools.beakerd INFO recipe ID 6 moved from Queued to Scheduled
Mar 20 07:38:13 beaker-server-lc beakerd[32477]: bkr.server.tools.beakerd DEBUG Selecting a system for recipe 7
Mar 20 07:38:13 beaker-server-lc beakerd[32477]: bkr.server.tools.beakerd DEBUG System : beaker-test-vm1 is available for Recipe 7
Mar 20 07:38:13 beaker-server-lc beakerd[32477]: bkr.server.model.scheduler DEBUG Reserving system beaker-test-vm1 for recipe 7
Mar 20 07:38:13 beaker-server-lc beakerd[32477]: bkr.server.model.activity DEBUG Tentative SystemActivity: object_id=1L, service=u'Scheduler', field=u'User', action=u'Reserved', old=u'', new=u'admin', user=admin
Mar 20 07:38:13 beaker-server-lc beakerd[32477]: bkr.server.model.inventory DEBUG Created reservation for system beaker-test-vm1 with type u'recipe', service u'Scheduler', user admin

Comment 4 Dan Callaghan 2017-03-30 03:23:03 UTC
Beaker 24.2 has been released.