Bug 199417 – [EWS] Multiple builds are triggered for one patch sometimes in new EWS

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Aakash Jain]

Noticed following duplicate builds for Patch 373348 in https://bugs.webkit.org/show_bug.cgi?id=199412 on all EWS queues:
mac:
https://ews-build.webkit.org/#/builders/7/builds/4506
https://ews-build.webkit.org/#/builders/7/builds/4507

ios:
https://ews-build.webkit.org/#/builders/2/builds/455
https://ews-build.webkit.org/#/builders/2/builds/456

webkitperl:
https://ews-build.webkit.org/#/builders/19/builds/4463
https://ews-build.webkit.org/#/builders/19/builds/4464


It seems like the 'Submit to new EWS' button was pressed almost at the same time when new EWS automatically fetched this patch. The code which submits the patch to Buildbot isn't completely thread-safe and it submitted the patch to EWS twice.

[Aakash Jain]

Logs from ews-app:
2019-07-02 11:48:55,938 - Saved patch in database, id: 373348
2019-07-02 11:48:55,941 - 1 r? patches, 1 patches need to be sent to Buildbot.
2019-07-02 11:48:55,942 - Starting new HTTPS connection (1): bugs.webkit.org
2019-07-02 11:48:56,307 - 1 r? patches, 1 patches need to be sent to Buildbot.
2019-07-02 11:48:56,308 - Starting new HTTPS connection (1): bugs.webkit.org
2019-07-02 11:48:57,218 - Saved build 1_28168 in database for patch_id: 373348
2019-07-02 11:48:57,288 - Saved build 1_28169 in database for patch_id: 373348
2019-07-02 11:48:57,393 - Saved build 1_28170 in database for patch_id: 373348
2019-07-02 11:48:57,446 - Patch 373348 already has bug id 199412 set.
2019-07-02 11:48:57,449 - Patch 373348 has already been sent to Buildbot.

[Aakash Jain]

*** Bug 200336 has been marked as a duplicate of this bug. ***

[Aakash Jain]

Dan reported that same issue also happened for https://bugs.webkit.org/show_bug.cgi?id=200324 ((attachment 375282 [details]) 
<https://ews-build.webkit.org/#/builders/15/builds/172>
and
<https://ews-build.webkit.org/#/builders/15/builds/173>

[Aakash Jain]

This issue also happens when someone clicks 'Submit to new EWS' button multiple times (before the status-bubbles loads).

[Aakash Jain]

I added additional logging to djnago app (ews-app).

'buildbot try' command (to send patch from djnago to buildbot) is taking 15-20 seconds, which is much higher than expected. Due to this after clicking 'Submit to new ews', it takes a while to load the status-bubbles. During this time, if someone clicks the button again, the patch is submitted again. 

Few logs:
2019-08-02 08:14:27,906 - took 15292ms to send patch 375409 to buildbot
2019-08-02 08:21:53,255 - took 16408ms to send patch 375410 to buildbot
2019-08-02 08:22:09,649 - took 27792ms to send patch 375411 to buildbot
2019-08-02 09:16:44,736 - took 15001ms to send patch 375412 to buildbot
2019-08-02 09:27:09,028 - took 16327ms to send patch 375308 to buildbot
2019-08-02 09:52:22,490 - took 15235ms to send patch 375414 to buildbot
2019-08-02 09:53:08,165 - took 17649ms to send patch 375415 to buildbot
2019-08-02 09:57:21,861 - took 15566ms to send patch 375416 to buildbot

[Aakash Jain]

The sent_to_buildbot flag is checked at the very beginning, and we set this flag after sending is successful. If user clicks twice, both threads/processes see sent_to_buildbot as false and send the patch to buildbot, and then both set sent_to_buildbot to True.

[Radar WebKit Bug Importer]

<rdar://problem/54980936>

[Aakash Jain]

Created attachment 381715 [details]
Patch

[Jonathan Bedard]

Comment on attachment 381715 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-app/ews/fetcher.py:63
> +        _log.info('{} r? patches, {} patches need to be sent to Buildbot: {}'.format(len(patch_ids), len(patches_to_send), patches_to_send))

What are 'patches_to_send' and what do they look like when printed?

[Aakash Jain]

Comment on attachment 381715 [details]
Patch

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

>> Tools/BuildSlaveSupport/ews-app/ews/fetcher.py:63
>> +        _log.info('{} r? patches, {} patches need to be sent to Buildbot: {}'.format(len(patch_ids), len(patches_to_send), patches_to_send))
> 
> What are 'patches_to_send' and what do they look like when printed?

patches_to_send is a list of r? patch_ids which needs EWS processing. 

This is just a log improvement. This is how the log looks like after this change:
29 r? patches, 2 patches need to be sent to Buildbot: [381699, 381700]

[Jonathan Bedard]

Comment on attachment 381715 [details]
Patch

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

>>> Tools/BuildSlaveSupport/ews-app/ews/fetcher.py:63
>>> +        _log.info('{} r? patches, {} patches need to be sent to Buildbot: {}'.format(len(patch_ids), len(patches_to_send), patches_to_send))
>> 
>> What are 'patches_to_send' and what do they look like when printed?
> 
> patches_to_send is a list of r? patch_ids which needs EWS processing. 
> 
> This is just a log improvement. This is how the log looks like after this change:
> 29 r? patches, 2 patches need to be sent to Buildbot: [381699, 381700]

Got it, so when we print patches, its just the patch IDs.

[WebKit Commit Bot]

Comment on attachment 381715 [details]
Patch

Clearing flags on attachment: 381715

Committed r251516: <https://trac.webkit.org/changeset/251516>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.