Closed Bug 572871 Opened 16 years ago Closed 15 years ago

Sisyphus - Crash Automation - crashtest job queue performance enhancements.

Categories

(Testing Graveyard :: Sisyphus, defect)

Other
Other
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bc, Assigned: bc)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
Bug 552681 added locks and skip values for the job queue as a way of dealing with contention between workers fighting over the next job. While this was an improvement and eliminated much of the conflict resolution code in checkSignatureWorker, the job queue lock became a bottle neck for serving jobs to workers. This became especially apparent when the number of urls per job was small and there were several workers competing for the same class of jobs. Using skip values for couchdb views was a problem in that the server had to read the view from the beginning of the range specified by the startkey and then skip over the specified number of rows before it could return the view. I thought that a better approach would be to use a startkey_docid to specify the actual row to be skipped. Using startkey_docid has the added advantage in that it can be updated for each viewdata as the jobs are processed in checkSignatureForWorker thus making the recording of the next starting point in the view automatic. This use of the startkey_docid and its sliding position in the job queue made possible the elimination of the job queue lock. Fixes: * viewdata dicts now contain information on the startkey and startkey_docid for each view. the viewdata skip value is used primarily as a flag to determine when a view has no rows (skip == -1), when the start of the job queue should be returned (skip == 0), or when the startkey/startkey_doci should be used to skip over the leading part of the job queue (skip == 1). update conflicts are minimized through the use of the different sliding job queue startkey_docid. In practice with up to 7 workers, there have been very few conflicts per thousands of served jobs. * non best matching workers would process jobs even though a different worker with the same operating system, operating system version and cpu type had already processed the job. Since it adds no value for repeated tests for the same "class" of worker, this was fixed to prevent "identical" workers from reprocessing jobs. * non best matching workers blocked after processing the chunk of 5 jobs served on each call to getSignatureForWorker. The fix is to allow the non-best matching workers to proceed through the pending job queue. Eliminating the job queue lock was crucial here since when the skip|startkey_docid reset occurs, the workers quickly read through the job queue looking for their new starting position. Requiring the worker to lock the queue while searching would lock out other workers for extended periods of time and reduce performance significantly. * to speed up job queue processing when startkey|startkey_docid are reset periodically, the job queue is now retrieved in chunks of 64 rather than 5. This only causes the request to return 2K of data which is insignificant and allows the job queue to be traversed quickly. * since locks are no longer used and we do not need to limit the time a worker has the queue locked, the worker can now scan through the entire job queue looking for the next job in one call. This significantly improves performance.
Attachment #452079 - Flags: review?(ctalbert)
Assignee: nobody → bclary
Status: NEW → ASSIGNED
Attachment #452079 - Flags: review?(ctalbert) → review+
Comment on attachment 452079 [details] [diff] [review] patch This takes some detailed reviewing, but once you start to see it, it's actually quite a bit simpler than what we had before. I like this change a lot.
Attached patch patch v2Splinter Review
Attachment #452079 - Attachment is obsolete: true
Attachment #453417 - Flags: review?(ctalbert)
Attachment #453417 - Flags: review?(ctalbert) → review+
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: