Bug 89725 - restructure manager/broker/worker code
Summary: restructure manager/broker/worker code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on: 90123 90125 90126 90408 90409 90510 90511 90513
Blocks: 76022 89267
  Show dependency treegraph
 
Reported: 2012-06-21 18:04 PDT by Dirk Pranke
Modified: 2012-07-12 15:24 PDT (History)
4 users (show)

See Also:


Attachments
Patch (71.78 KB, patch)
2012-06-21 18:05 PDT, Dirk Pranke
no flags Details | Formatted Diff | Diff
Patch (66.50 KB, patch)
2012-06-21 21:08 PDT, Dirk Pranke
no flags Details | Formatted Diff | Diff
roll-up patch based off of r121304; not for review or landing (67.63 KB, patch)
2012-06-26 19:01 PDT, Dirk Pranke
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2012-06-21 18:04:43 PDT
restructure manager/broker/worker code
Comment 1 Dirk Pranke 2012-06-21 18:05:29 PDT
Created attachment 148931 [details]
Patch
Comment 2 Dirk Pranke 2012-06-21 21:08:26 PDT
Created attachment 148956 [details]
Patch
Comment 3 Dirk Pranke 2012-06-26 19:01:39 PDT
Created attachment 149665 [details]
roll-up patch based off of r121304; not for review or landing

attaching a patch that basically shows what manager.py, manager_worker_broker.py, and worker.py will look like after I've cleaned up the multiprocessing code. This patch needs to be broken up into reviewable chunks and some of the unit tests need to be redone, so don't bother to review it.
Comment 4 Dirk Pranke 2012-06-26 19:06:59 PDT
Also, note that "manager_worker_broker", although never a good name, is now a really bad name.

I am trying to structure the code like the multiprocessing.Pool interface and the concurrency.futures.ProcessExecutor interface. The interfaces is much simpler though, as all we provide is a single run([list of things]) call. We could provide more flexibility easily, but it isn't needed at this time.

I suggest we use something like MessagePool or MessagePoolExecutor or something like that ...

To recap why we're not just using one of those two interfaces ...

The workers are stateful, and those two interfaces assume each task is stateless. We could probably work around this using globals and restarting as necessary, but that would make things kinda ugly.

There isn't a 1:1 mapping between tasks and messages, since we group tests into shards: running one shard generates N results for tests and we want those results as they occur

As an aside using messages instead of request/reply tasks allows us to handle errors better, log messages, better, and generate better stats.
Comment 5 Adam Barth 2012-06-26 21:03:34 PDT
I'm slightly unsure where to start looking at these patches.  The rollup patches are pretty big.  :)

Maybe I should wait for you to mark one of the patches for review.
Comment 6 Dirk Pranke 2012-06-27 09:45:33 PDT
(In reply to comment #5)
> I'm slightly unsure where to start looking at these patches.  The rollup patches are pretty big.  :)
> 
> Maybe I should wait for you to mark one of the patches for review.

absolutely, I don't want/expect you to review these. this stuff is just in case you wanted to apply the patches and take a look at the end state(s).
Comment 7 Dirk Pranke 2012-07-12 15:24:22 PDT
all patches have been landed.