Bug 101407 – garden-o-matic should handle concurrent requests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dirk Pranke]

garden-o-matic should handle concurrent requests

[Dirk Pranke]

Created attachment 172671 [details]
Patch

[Dirk Pranke]

Comment on attachment 172671 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=172671&action=review

> Tools/Scripts/webkitpy/tool/servers/gardeningserver.py:-87
> -

Note that this routine was duplicated and we want the version below on line 103.

[Adam Barth]

It's hard to forsee the consequences of this change.  What if multiple requests want to touch TestExpectations at the same time?

[Dirk Pranke]

(In reply to comment #3)
> It's hard to forsee the consequences of this change. 

Agreed. I *think* it's safe, at least as far as I wrote in the changelog. I'm open to other suggestions; we can make the local file access go through file:/// urls, at some cost in complexity (I don't really like mixing file:/// and http://localhost/ requests), but I think we need to enable issuing multiple rebaseline commands concurrently (pipelined, really) or else you have to wait for one to complete before starting another, and that adds a lot of latency.

I say we give this a shot and see what happens.

> What if multiple requests want to touch TestExpectations at the same time?

In http://trac.webkit.org/changeset/133508, I realized we already had this problem by attempting to run rebaseline-test-internal in parallel (which itself will update the expectations) and I fixed that by synchronizing on a file lock per each TestExpectations file. So, in theory we should just contend over intervals for rewriting the file, which should be pretty fast

(I haven't verified this yet, I'm in the middle of adding more logging to better understand the performance).

[Ojan Vafai]

Comment on attachment 172671 [details]
Patch

I think this is safe for everything we currently use garden-o-matic for (i.e. rebaselining). In the future if we add things like rollout support, we'll need to be careful to only do so when there are no other request in progress. I'm pretty sure all the other stuff in the gardening server (e.g. rollout) is dead code at this point.

[Adam Barth]

Ok.  You two break it, you own it.  :)

[Dirk Pranke]

(In reply to comment #6)
> Ok.  You two break it, you own it.  :)

It might be nice to have this configurable behind a command line flag, just in case ... I'll have to look into how doable that is.

[Dirk Pranke]

Committed r133804: <http://trac.webkit.org/changeset/133804>