Created attachment 372104 [details] Number 2 Twelve Hours Later! I posted attachment #372061 [details] (bug #198834) at 10 am PDT today (06/13/2019). **12** hours later the patch is still listed as #2 on all New EWS queues! Old EWS long since processed the patch.
<rdar://problem/51735036>
This one looks like some network issue (in communication between ews-app and buildbot) and probably some super corner case. ews-app thinks it has sent the patch to Buildbot, but Buildbot never seems to have received it. Logs from ews-app: 2019-06-13 10:00:35,320 - Logging in as ews-feeder@webkit.org... 2019-06-13 10:00:35,441 - Getting list of patches needing review, URL: https://bugs.webkit.org/request.cgi?action=queue&type=review&group=type 2019-06-13 10:00:36,357 - Saved patch in database, id: 372061 2019-06-13 10:00:36,486 - 43 r? patches, 1 patches need to be sent to Buildbot. 2019-06-13 10:00:36,487 - Starting new HTTPS connection (1): bugs.webkit.org 2019-06-13 10:01:07,275 - Logging in as ews-feeder@webkit.org... Patch is also present in /tmp/372061.patch (so there was no network/permission issue in downloading the patch). Logs from twistd.log on Buildbot (ews-build): 2019-06-13 09:55:45-0700 [-] Splitting long line: /Applications/Xcode.app/Co 6258 (not warning anymore for this log) 2019-06-13 09:56:40-0700 [-] Splitting long line: /Applications/Xcode.app/Co 4444 (not warning anymore for this log) 2019-06-13 10:02:11-0700 [Broker,500,172.31.50.61] user buildbot_try requesting build on builders None 2019-06-13 10:02:11-0700 [-] added buildset 9980 to database No logs at 2019-06-13 10:00:36 (Note that buildset 9980 at 10:02:11 is for another patch). Will look into it more.
I suspect this issue has something to do with large size of the patch (665 kb).
Yes, the issue due to Patch size (over 640kb). Manually running 'buildbot try' command results in below error. Also 'buildbot try' returns with exit code zero (even though it failed), that's why ews-app thinks it successful sent the patch to buildbot. Filed: https://github.com/buildbot/buildbot/issues/4837 Traceback (most recent call last): File "/usr/lib64/python2.7/site-packages/twisted/spread/pb.py", line 563, in expressionReceived method(*sexp[1:]) File "/usr/lib64/python2.7/site-packages/twisted/spread/pb.py", line 933, in proto_answer d.callback(self.unserialize(netResult)) File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 393, in callback self._startRunCallbacks(result) File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 501, in _startRunCallbacks self._runCallbacks() --- <exception caught here> --- File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 587, in _runCallbacks current.result = callback(current.result, *args, **kw) File "/usr/lib/python2.7/site-packages/buildbot/clients/tryclient.py", line 702, in _deliverJob_pb self.config.get('properties', {})) File "/usr/lib64/python2.7/site-packages/twisted/spread/pb.py", line 343, in callRemote _name, args, kw) File "/usr/lib64/python2.7/site-packages/twisted/spread/pb.py", line 871, in _sendMessage self.sendCall(prefix+"message", requestID, objectID, message, answerRequired, netArgs, netKw) File "/usr/lib64/python2.7/site-packages/twisted/spread/pb.py", line 584, in sendCall self.sendEncoded(exp) File "/usr/lib64/python2.7/site-packages/twisted/spread/banana.py", line 320, in sendEncoded self._encode(obj, encodeStream.write) File "/usr/lib64/python2.7/site-packages/twisted/spread/banana.py", line 333, in _encode self._encode(elem, write) File "/usr/lib64/python2.7/site-packages/twisted/spread/banana.py", line 333, in _encode self._encode(elem, write) File "/usr/lib64/python2.7/site-packages/twisted/spread/banana.py", line 333, in _encode self._encode(elem, write) File "/usr/lib64/python2.7/site-packages/twisted/spread/banana.py", line 362, in _encode "byte string is too long to send (%d)" % (len(obj),)) twisted.spread.banana.BananaError: byte string is too long to send (681011) [ews@ews ews-app]$ echo $? 0
The error message comes from: https://github.com/twisted/twisted/blob/trunk/src/twisted/spread/banana.py#L364 And the limit is defined in: https://github.com/twisted/twisted/blob/trunk/src/twisted/spread/banana.py#L91 SIZE_LIMIT = 640 * 1024 # 640k is all you'll ever need :-)
Fix, please?
Oh, bug is upstream. That really sucks
(In reply to Daniel Bates from comment #6) > Fix, please? Fix coming soon.
Created attachment 372157 [details] Patch
(In reply to Aakash Jain from comment #9) > Created attachment 372157 [details] > Patch This is similar to old fix in: <rdar://problem/33870706>.
This is also as suggested in http://trac.buildbot.net/ticket/2123#comment:2
5 MB seems too small as well, especially given that EWS gets into a strange state when seeming a large patch. I’m not sure what was the largest patch we had, but I won’t be surprised if it was over 100 MB.
Created attachment 372247 [details] Patch
(In reply to Alexey Proskuryakov from comment #12) > 5 MB seems too small as well, especially given that EWS gets into a strange state when seeming a large patch. > > I’m not sure what was the largest patch we had, but I won’t be surprised if it was over 100 MB. Increased the limit to 100 MB. Also, this fix is at receiving end (buildbot). I need a similar fix on ews-app (django app) side, which invokes the 'buildbot try' command. I can't simply use this approach (importing banana and changing SIZE_LIMIT) in ews-app, since it invokes 'buildbot try' command using subprocess.call(). So I need a command-line/environment fix, instead of python fix. I will tackle it separately. For now, I have manually increased the limit on that machine (in /usr/lib64/python2.7/site-packages/twisted/spread/banana.py)
I'd like to add some information coming from an IRC exchange Aakash and I had. According to https://github.com/twisted/twisted/blob/trunk/src/twisted/spread/banana.py#L364, our patches are basically being held as strings. This is why there is an upper limit. Ultimately, we aren't going to be able to get rid of the upper limit, and verging into the multi-gigabyte range could be problematic. Ultimately, we just need to pay attention to this and bump the number again if we need to. We had an old version of patch-based EWS internally which used 2 MB, and I don't recall hearing any complaints, so I suspect that 100 MB will be fine.
Comment on attachment 372247 [details] Patch Clearing flags on attachment: 372247 Committed r246498: <https://trac.webkit.org/changeset/246498>
All reviewed patches have been landed. Closing bug.
Largest patch I have seen in a while: 50MB in https://bugs.webkit.org/show_bug.cgi?id=209137