214257 2020-07-13 08:18:27 -0700 Delete old ews script DumpRenderTreeWatchDog.py 2020-07-15 14:04:04 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified ASSIGNED https://bugs.webkit.org/show_bug.cgi?id=196567 InRadar P2 Normal --- 1 aakash_jain aakash_jain ap darin jbedard pvollan webkit-bot-watchers-bugzilla webkit-bug-importer oldest_to_newest 1671030 0 aakash_jain 2020-07-13 08:18:27 -0700 We should delete old ews script DumpRenderTreeWatchDog.py. This script was used on Windows bots in old EWS to terminate hanging DRT processes. In new EWS, buildbot terminate the build automatically after 20 minute of inactivity. Further, in new EWS builds, we also run kill-old-processes which kills DumpRenderTree.exe and imagediff.exe Therefore, we don't need this script anymore and it should be deleted. 1671031 1 webkit-bug-importer 2020-07-13 08:19:48 -0700 <rdar://problem/65477036> 1671032 2 404148 aakash_jain 2020-07-13 08:20:02 -0700 Created attachment 404148 Patch 1671036 3 aakash_jain 2020-07-13 08:29:51 -0700 This script (dumprendertreewatchdog.py) killed "ImageDiff.exe", while kill-old-processes kills "imagediff.exe" (https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/kill-old-processes#L45). Not sure if the capitalization matters. 1671046 4 pvollan 2020-07-13 09:14:17 -0700 (In reply to Aakash Jain from comment #3) > This script (dumprendertreewatchdog.py) killed "ImageDiff.exe", while > kill-old-processes kills "imagediff.exe" > (https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/kill- > old-processes#L45). Not sure if the capitalization matters. I think this script is still in use on the EWS bots, or am I mistaken? 1671048 5 aakash_jain 2020-07-13 09:19:55 -0700 (In reply to Per Arne Vollan from comment #4) > I think this script is still in use on the EWS bots, or am I mistaken? Some EWS bots were using it by mistake, I just stopped all that. See <rdar://problem/65142015>. Is there a reason that this script is required on EWS bots? Buildbot already terminates any hanging build, and we use kill-old-processes to kill DumpRenderTree.exe in the beginning of every build. 1671052 6 pvollan 2020-07-13 09:27:36 -0700 (In reply to Aakash Jain from comment #5) > (In reply to Per Arne Vollan from comment #4) > > I think this script is still in use on the EWS bots, or am I mistaken? > Some EWS bots were using it by mistake, I just stopped all that. See > <rdar://problem/65142015>. > > Is there a reason that this script is required on EWS bots? Buildbot already > terminates any hanging build, and we use kill-old-processes to kill > DumpRenderTree.exe in the beginning of every build. Yes, I believe this is still required. The script will kill hanging processes during the test run, making sure the test run completes. I believe we have had cases where the test run never finishes, which is fixed by having this script running. I don't believe the script kill-old-processes supports this. 1671054 7 darin 2020-07-13 09:32:50 -0700 Then it seems like a bad idea to stop running it, and a bad idea to delete it. 1671614 8 jbedard 2020-07-14 17:25:45 -0700 If we think such a script is needed, why don't we also need it on build.webkit.org? One of the purposes of the push to move EWS to buildbot was so that EWS was running with the same configuration as trunk testing. I'm pretty sure trunk deals with the hanging problem by leaning on run-webkit-tests to take care of it and, as a last resort, buildbot timing out. 1671665 9 ap 2020-07-14 22:56:43 -0700 I think that the new behavior is: 1. Buildbot will terminate unresponsive steps. 2. kill-old-processes will be executed before the next test to clean up. So we should be fine? In any case, this is dead code now that Aakash removed the scheduled task from all bots where we still had it. 1671886 10 pvollan 2020-07-15 13:04:12 -0700 (In reply to Jonathan Bedard from comment #8) > If we think such a script is needed, why don't we also need it on > build.webkit.org? > > One of the purposes of the push to move EWS to buildbot was so that EWS was > running with the same configuration as trunk testing. I'm pretty sure trunk > deals with the hanging problem by leaning on run-webkit-tests to take care > of it and, as a last resort, buildbot timing out. Yes, we should also have it on build.webkit.org (or a similar feature), since the same issue will be present there. However, it is more important on EWS, since without it, Windows EWS will often start lagging behind, and we loose test coverage. 1671898 11 pvollan 2020-07-15 13:22:19 -0700 (In reply to Alexey Proskuryakov from comment #9) > I think that the new behavior is: > > 1. Buildbot will terminate unresponsive steps. > 2. kill-old-processes will be executed before the next test to clean up. > > So we should be fine? In any case, this is dead code now that Aakash removed > the scheduled task from all bots where we still had it. The script is able to unblock the waiting parent process of a stuck DumpRenderTree/ImageDiff while running the layout tests, making the test run succeed, although with some test failures. It seems buildbot will terminate the whole step, which means the test run would fail, or am I mistaken? Anyway, I see that this is dead code now after moving to buildbot. If we still need this feature, maybe it should be part of the run-webkit-tests script? Also, I haven't observed hangs after the buildbot move, which makes me think that this feature is not super critical at this point, since the Windows EWS bots are keeping up with the queue. Great job! 1671927 12 jbedard 2020-07-15 14:04:04 -0700 (In reply to Per Arne Vollan from comment #11) > (In reply to Alexey Proskuryakov from comment #9) > > I think that the new behavior is: > > > > 1. Buildbot will terminate unresponsive steps. > > 2. kill-old-processes will be executed before the next test to clean up. > > > > So we should be fine? In any case, this is dead code now that Aakash removed > > the scheduled task from all bots where we still had it. > > The script is able to unblock the waiting parent process of a stuck > DumpRenderTree/ImageDiff while running the layout tests, making the test run > succeed, although with some test failures. It seems buildbot will terminate > the whole step, which means the test run would fail, or am I mistaken? > Anyway, I see that this is dead code now after moving to buildbot. If we > still need this feature, maybe it should be part of the run-webkit-tests > script? Also, I haven't observed hangs after the buildbot move, which makes > me think that this feature is not super critical at this point, since the > Windows EWS bots are keeping up with the queue. Great job! Yes, I would expect such a script to be part of run-webkit-tests, if we needed it. Seems like it would basically turn an error that broke the entire test suite into one that just broke a few tests. Although such behavior would definitely cause flakey test failures. 404148 2020-07-13 08:20:02 -0700 2020-07-13 09:18:48 -0700 Patch bug-214257-20200713112001.patch text/plain 3238 aakash_jain SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI2NDMwMCkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDEyIEBACisyMDIwLTA3LTEzICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNv bT4KKworICAgICAgICBEZWxldGUgb2xkIGV3cyBzY3JpcHQgRHVtcFJlbmRlclRyZWVXYXRjaERv Zy5weQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjE0 MjU3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBF V1NUb29scy9kdW1wcmVuZGVydHJlZXdhdGNoZG9nLnB5OiBSZW1vdmVkLgorCiAyMDIwLTA3LTEz ICBaYW4gRG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4KIAogICAgICAgICBbV1BFXSBT dGljayB0byBkZWZhdWx0IEdsaWIgc291cmNlIHByaW9yaXRpZXMgaW4gSGVhZGxlc3NWaWV3QmFj a2VuZCwgV2luZG93Vmlld0JhY2tlbmQKSW5kZXg6IFRvb2xzL0VXU1Rvb2xzL2R1bXByZW5kZXJ0 cmVld2F0Y2hkb2cucHkKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gVG9vbHMvRVdTVG9vbHMvZHVtcHJlbmRlcnRy ZWV3YXRjaGRvZy5weQkocmV2aXNpb24gMjY0MzAwKQorKysgVG9vbHMvRVdTVG9vbHMvZHVtcHJl bmRlcnRyZWV3YXRjaGRvZy5weQkobm9uZXhpc3RlbnQpCkBAIC0xLDUzICswLDAgQEAKLSMgQ29w eXJpZ2h0IChDKSAyMDE5IEFwcGxlIEluYy4gQWxsIHJpZ2h0cyByZXNlcnZlZC4KLSMKLSMgUmVk aXN0cmlidXRpb24gYW5kIHVzZSBpbiBzb3VyY2UgYW5kIGJpbmFyeSBmb3Jtcywgd2l0aCBvciB3 aXRob3V0Ci0jIG1vZGlmaWNhdGlvbiwgYXJlIHBlcm1pdHRlZCBwcm92aWRlZCB0aGF0IHRoZSBm b2xsb3dpbmcgY29uZGl0aW9ucyBhcmUKLSMgbWV0OgotIwotIyAgICAgKiBSZWRpc3RyaWJ1dGlv bnMgb2Ygc291cmNlIGNvZGUgbXVzdCByZXRhaW4gdGhlIGFib3ZlIGNvcHlyaWdodAotIyBub3Rp Y2UsIHRoaXMgbGlzdCBvZiBjb25kaXRpb25zIGFuZCB0aGUgZm9sbG93aW5nIGRpc2NsYWltZXIu Ci0jICAgICAqIFJlZGlzdHJpYnV0aW9ucyBpbiBiaW5hcnkgZm9ybSBtdXN0IHJlcHJvZHVjZSB0 aGUgYWJvdmUKLSMgY29weXJpZ2h0IG5vdGljZSwgdGhpcyBsaXN0IG9mIGNvbmRpdGlvbnMgYW5k IHRoZSBmb2xsb3dpbmcgZGlzY2xhaW1lcgotIyBpbiB0aGUgZG9jdW1lbnRhdGlvbiBhbmQvb3Ig b3RoZXIgbWF0ZXJpYWxzIHByb3ZpZGVkIHdpdGggdGhlCi0jIGRpc3RyaWJ1dGlvbi4KLSMKLSMg VEhJUyBTT0ZUV0FSRSBJUyBQUk9WSURFRCBCWSBUSEUgQ09QWVJJR0hUIEhPTERFUlMgQU5EIENP TlRSSUJVVE9SUwotIyAiQVMgSVMiIEFORCBBTlkgRVhQUkVTUyBPUiBJTVBMSUVEIFdBUlJBTlRJ RVMsIElOQ0xVRElORywgQlVUIE5PVAotIyBMSU1JVEVEIFRPLCBUSEUgSU1QTElFRCBXQVJSQU5U SUVTIE9GIE1FUkNIQU5UQUJJTElUWSBBTkQgRklUTkVTUyBGT1IKLSMgQSBQQVJUSUNVTEFSIFBV UlBPU0UgQVJFIERJU0NMQUlNRUQuIElOIE5PIEVWRU5UIFNIQUxMIFRIRSBDT1BZUklHSFQKLSMg T1dORVIgT1IgQ09OVFJJQlVUT1JTIEJFIExJQUJMRSBGT1IgQU5ZIERJUkVDVCwgSU5ESVJFQ1Qs IElOQ0lERU5UQUwsCi0jIFNQRUNJQUwsIEVYRU1QTEFSWSwgT1IgQ09OU0VRVUVOVElBTCBEQU1B R0VTIChJTkNMVURJTkcsIEJVVCBOT1QKLSMgTElNSVRFRCBUTywgUFJPQ1VSRU1FTlQgT0YgU1VC U1RJVFVURSBHT09EUyBPUiBTRVJWSUNFUzsgTE9TUyBPRiBVU0UsCi0jIERBVEEsIE9SIFBST0ZJ VFM7IE9SIEJVU0lORVNTIElOVEVSUlVQVElPTikgSE9XRVZFUiBDQVVTRUQgQU5EIE9OIEFOWQot IyBUSEVPUlkgT0YgTElBQklMSVRZLCBXSEVUSEVSIElOIENPTlRSQUNULCBTVFJJQ1QgTElBQklM SVRZLCBPUiBUT1JUCi0jIChJTkNMVURJTkcgTkVHTElHRU5DRSBPUiBPVEhFUldJU0UpIEFSSVNJ TkcgSU4gQU5ZIFdBWSBPVVQgT0YgVEhFIFVTRQotIyBPRiBUSElTIFNPRlRXQVJFLCBFVkVOIElG IEFEVklTRUQgT0YgVEhFIFBPU1NJQklMSVRZIE9GIFNVQ0ggREFNQUdFLgotCi1pbXBvcnQgb3MK LWltcG9ydCBzdWJwcm9jZXNzCi1pbXBvcnQgdGltZQotCi1EVU1QX1BJRCA9IDAKLURVTVBfU1RB UlRfVElNRSA9IDAKLQotd2hpbGUoMSk6Ci0gICAgdGltZS5zbGVlcCgxMCkKLSAgICBDVVJfVElN RSA9IGludCh0aW1lLnRpbWUoKSkKLQotICAgIHBpZF9mZXRjaCA9IHN1YnByb2Nlc3MuUG9wZW4o InBzIC1lIHwgZ3JlcCBcIkR1bXBSZW5kZXJUcmVlXCIgfCBhd2sgJ3twcmludCAkMX0nIiwgc2hl bGw9VHJ1ZSwgc3Rkb3V0PXN1YnByb2Nlc3MuUElQRSwpCi0gICAgQ1VSUkVOVF9QSUQgPSBwaWRf ZmV0Y2guY29tbXVuaWNhdGUoKVswXS5zcGxpdCgnXG4nKVswXQotICAgIENIRUNLRUQgPSAxCi0g ICAgIyBXZSBvbmx5IGNhcmUgYWJvdXQgZG9pbmcgYW55dGhpbmcgaWYgdGhlcmUgaXMgYSBEdW1w UmVuZGVyVHJlZSBydW5uaW5nCi0gICAgaWYgKENVUlJFTlRfUElEKToKLSAgICAgICAgIyBJZiB0 aGUgUElEIGhhcyBjaGFuZ2VkLCByZXNldCB0aGUgdGltZXIgYW5kIHRoZSBhY3RpdmUgcGlkCi0g ICAgICAgIGlmIChEVU1QX1BJRCAhPSBDVVJSRU5UX1BJRCk6Ci0gICAgICAgICAgICBEVU1QX1BJ RCA9IENVUlJFTlRfUElECi0gICAgICAgICAgICBEVU1QX1NUQVJUX1RJTUUgPSB0aW1lLnRpbWUo KQotICAgICAgICBlbHNlOgotICAgICAgICAjIElmIHRoZSBhY3RpdmUgYW5kIGN1cnJlbnQgUElE IGFyZSB0aGUgc2FtZSBhbmQgYW4gaG91ciBoYXMgZWxhcHNlZCwga2lsbCBEdW1wUmVuZGVyVHJl ZSBhbmQgcmVzZXQgdGhlIGFjdGl2ZSBQSUQgdG8gMAotICAgICAgICAgICAgaWYgKENVUl9USU1F IC0gRFVNUF9TVEFSVF9USU1FID49IDM2MDApOgotICAgICAgICAgICAgICAgIG9zLnN5c3RlbSgi dGFza2tpbGwgL0YgL0lNIER1bXBSZW5kZXJUcmVlLmV4ZSIpCi0gICAgICAgICAgICAgICAgb3Mu c3lzdGVtKCJ0YXNra2lsbCAvRiAvSU0gSW1hZ2VEaWZmLmV4ZSIpCi0gICAgICAgICAgICAgICAg RFVNUF9QSUQgPSAwCi0gICAgZWxzZToKLSAgICAgICAgRFVNUF9QSUQgPSAwCg==