101407 2012-11-06 16:11:58 -0800 garden-o-matic should handle concurrent requests 2012-11-07 13:42:52 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 dpranke dpranke abarth dglazkov ojan oldest_to_newest 760160 0 dpranke 2012-11-06 16:11:58 -0800 garden-o-matic should handle concurrent requests 760166 1 172671 dpranke 2012-11-06 16:14:43 -0800 Created attachment 172671 Patch 760169 2 172671 dpranke 2012-11-06 16:16:42 -0800 Comment on attachment 172671 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. 760171 3 abarth 2012-11-06 16:17:12 -0800 It's hard to forsee the consequences of this change. What if multiple requests want to touch TestExpectations at the same time? 760185 4 dpranke 2012-11-06 16:25:32 -0800 (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). 760193 5 172671 ojan 2012-11-06 16:31:33 -0800 Comment on attachment 172671 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. 760235 6 abarth 2012-11-06 17:27:15 -0800 Ok. You two break it, you own it. :) 760238 7 dpranke 2012-11-06 17:30:33 -0800 (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. 761024 8 dpranke 2012-11-07 13:42:52 -0800 Committed r133804: <http://trac.webkit.org/changeset/133804> 172671 2012-11-06 16:14:43 -0800 2012-11-06 16:31:33 -0800 Patch bug-101407-20121106161255.patch text/plain 2936 dpranke U3VidmVyc2lvbiBSZXZpc2lvbjogMTMzNjc4CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggOTFiMmY5ZTRiZjYzY2M5OTIxN2IwNWIzMmUyMmE5YzM2 MDFkMTllZS4uNzU3NzFjNGM4M2RlZTcwNjljNmVjZDM5ODIxOGRiODkyMDBmNzk4MiAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDMw IEBACisyMDEyLTExLTA2ICBEaXJrIFByYW5rZSAgPGRwcmFua2VAY2hyb21pdW0ub3JnPgorCisg ICAgICAgIGdhcmRlbi1vLW1hdGljIHNob3VsZCBoYW5kbGUgY29uY3VycmVudCByZXF1ZXN0cwor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTAxNDA3CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgR2FyZGVuLW8t TWF0aWMgaXMgaW1wbGVtZW50ZWQgb24gdG9wIG9mIHRoZSBCYXNlSHR0cFNlcnZlcgorICAgICAg ICBweXRob24gbGlicmFyeSwgd2hpY2ggaW1wbGVtZW50cyBhIHNpbmdsZS10aHJlYWRlZCAob25l IHJlcXVlc3QgYXQKKyAgICAgICAgYSB0aW1lKSBzZXJ2ZXIuIFRoaXMgbWVhbnMgdGhhdCB3ZSBj YW4gb25seSByZWJhc2VsaW5lIG9uZSB0ZXN0IGF0CisgICAgICAgIGEgdGltZSwgYW5kIHdoZW4g dXNpbmcgYSBsb2NhbCByZXN1bHRzIGZpbGUsIGV2ZW4gZmV0Y2hpbmcgdGhlIHJlc3VsdHMKKyAg ICAgICAgZm9yIGRpZmZzIGlzIHNpbmdsZS10aHJlYWRlZC4KKworICAgICAgICBNaXhpbmcgaW4g dGhlIFRocmVhZGluZ01peGluIGVuYWJsZXMgY29uY3VycmVudCByZXF1ZXN0cyAoc3ViamVjdAor ICAgICAgICB0byB0aGUgZ2xvYmFsIHB5dGhvbiBpbnRlcnByZXRlciBsb2NrKS4gR2l2ZW4gdGhh dCB3ZSdyZSBkb2luZyBhbGwKKyAgICAgICAgb2YgdGhlIHdvcmsgaW4gd2Via2l0LXBhdGNoIHN1 YnByb2Nlc3NlcywgSSB0aGluayB0aGlzIGlzIHNhZmUuCisKKyAgICAgICAgT2YgY291cnNlLCBp dCBtYXkgbm90IGJlIHNhZmUgdG8gZG8gc29tZSBvZiB0aGUgc3VicHJvY2Vzc2VzIGluCisgICAg ICAgIHBhcmFsbGVsIChlLmcuLCByb2xsb3V0KSwgYnV0IEkgdGhpbmsgYXMgbG9uZyBhcyB3ZSBk b24ndCBpc3N1ZQorICAgICAgICBjb21tYW5kcyB0aGF0IHRvdWNoIG92ZXJsYXBwaW5nIHNldHMg b2YgZmlsZXMgdGhpbmdzIHNob3VsZCBiZSBmaW5lLgorICAgICAgICBUaGUgZmlsZSBsb2NraW5n IG92ZXIgbW9kaWZ5aW5nIHRlc3RleHBlY3RhdGlvbnMgc2hvdWxkIGtlZXAgdGhhdAorICAgICAg ICBmcm9tIGJlaW5nIGNvcnJ1cHRlZC4KKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvdG9v bC9zZXJ2ZXJzL2dhcmRlbmluZ3NlcnZlci5weToKKyAgICAgICAgKEdhcmRlbmluZ0hUVFBTZXJ2 ZXIpOgorICAgICAgICAoR2FyZGVuaW5nSFRUUFJlcXVlc3RIYW5kbGVyKToKKwogMjAxMi0xMS0w NiAgUmFwaGFlbCBLdWJvIGRhIENvc3RhICA8cmFwaGFlbC5rdWJvLmRhLmNvc3RhQGludGVsLmNv bT4KIAogICAgICAgICBbV1RSXSBSZXNldCBFdmVudFNlbmRlciBiZWZvcmUgcnVubmluZyBlYWNo IHRlc3QuCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3Rvb2wvc2VydmVycy9n YXJkZW5pbmdzZXJ2ZXIucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L3Rvb2wvc2VydmVycy9n YXJkZW5pbmdzZXJ2ZXIucHkKaW5kZXggZTU3YWZmMTE5YWRjZmJmYmJhZTZhNDhmNmRkN2RjODZi MmJiMTUxNy4uZTg5YzJhMDhhZTBmMjRlMjQ4YzQxMDkzOTQyMTEyNzE3MDIxNjcxMyAxMDA2NDQK LS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS90b29sL3NlcnZlcnMvZ2FyZGVuaW5nc2VydmVy LnB5CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvdG9vbC9zZXJ2ZXJzL2dhcmRlbmluZ3Nl cnZlci5weQpAQCAtMjMsNiArMjMsNyBAQAogIyBPRiBUSElTIFNPRlRXQVJFLCBFVkVOIElGIEFE VklTRUQgT0YgVEhFIFBPU1NJQklMSVRZIE9GIFNVQ0ggREFNQUdFLgogCiBpbXBvcnQgQmFzZUhU VFBTZXJ2ZXIKK2ltcG9ydCBTb2NrZXRTZXJ2ZXIKIGltcG9ydCBsb2dnaW5nCiBpbXBvcnQganNv bgogaW1wb3J0IG9zCkBAIC01Miw3ICs1Myw3IEBAIGNsYXNzIEJ1aWxkQ292ZXJhZ2VFeHRyYXBv bGF0b3Iob2JqZWN0KToKICAgICAgICAgcmV0dXJuIHNlbGYuX2NvdmVyZWRfdGVzdF9jb25maWd1 cmF0aW9uc19mb3JfYnVpbGRlcl9uYW1lKClbYnVpbGRlcl9uYW1lXQogCiAKLWNsYXNzIEdhcmRl bmluZ0hUVFBTZXJ2ZXIoQmFzZUhUVFBTZXJ2ZXIuSFRUUFNlcnZlcik6CitjbGFzcyBHYXJkZW5p bmdIVFRQU2VydmVyKFNvY2tldFNlcnZlci5UaHJlYWRpbmdNaXhJbiwgQmFzZUhUVFBTZXJ2ZXIu SFRUUFNlcnZlcik6CiAgICAgZGVmIF9faW5pdF9fKHNlbGYsIGh0dHBkX3BvcnQsIGNvbmZpZyk6 CiAgICAgICAgIHNlcnZlcl9uYW1lID0gJycKICAgICAgICAgc2VsZi50b29sID0gY29uZmlnWyd0 b29sJ10KQEAgLTgyLDkgKzgzLDYgQEAgY2xhc3MgR2FyZGVuaW5nSFRUUFJlcXVlc3RIYW5kbGVy KFJlZmxlY3Rpb25IYW5kbGVyKToKICAgICBhbGxvd19jcm9zc19vcmlnaW5fcmVxdWVzdHMgPSBU cnVlCiAgICAgZGVidWdfb3V0cHV0ID0gJycKIAotICAgIGRlZiBfcnVuX3dlYmtpdF9wYXRjaChz ZWxmLCBhcmdzKToKLSAgICAgICAgcmV0dXJuIHNlbGYuc2VydmVyLnRvb2wuZXhlY3V0aXZlLnJ1 bl9jb21tYW5kKFtzZWxmLnNlcnZlci50b29sLnBhdGgoKV0gKyBhcmdzLCBjd2Q9c2VsZi5zZXJ2 ZXIudG9vbC5zY20oKS5jaGVja291dF9yb290KQotCiAgICAgZGVmIHJvbGxvdXQoc2VsZik6CiAg ICAgICAgIHJldmlzaW9uID0gc2VsZi5xdWVyeVsncmV2aXNpb24nXVswXQogICAgICAgICByZWFz b24gPSBzZWxmLnF1ZXJ5WydyZWFzb24nXVswXQo=