Bug 209481

The logic to calculate the position in queue is: 1) define a start time (e.g.: 3 days back from current time) 2) count number of patches sent to buildbot from star-time and patch-modified-time. 3) get all builds on a queue after start-time and find list of patches from those builds 4) position_in_queue = output of #2 - output of #3 Earlier the patch-modified-time was same as the timestamp when the patch was sent to buildbot. Now we also have sent_to_commit_queue, and patch-modified-time can be the timestamp when the patch was sent-to-commit-queue. Therefore by using patch-modified-time, we might be taking a larger time-range, and there would be more patches included in #2, and some of those patches might have been processed by rest of ews queues before the start-time, and later-on by commit-queue. Using created timestamp instead of modified timestamp of patch should fix this issue.

Created attachment 394376 [details] Patch

Comment on attachment 394376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394376&action=review > Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:276 > + if patch.created < from_timestamp: Confused what this has to do with the bug title.

Comment on attachment 394376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394376&action=review >> Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:276 >> + if patch.created < from_timestamp: > > Confused what this has to do with the bug title. Consider the scenario in which: - a new queue 'X' was added 2 days back (with StatusBubble.DAYS_TO_CHECK = 3) - a patch was uploaded 4 days back, and so it would never be processed on the new queue 'X', and so the bubble for 'X' should NOT be displayed. - Now let's say the patch was cq+ today, and so the patch-modified-time would be updated to today. This would result in incorrectly showing the bubble for queue 'X' (with position #1) instead of hiding the bubble for 'X'. This is because the logic was based on patch-modified-time. Therefore patch created time is a better indicator. Also note that the 'created' timestamp is from django database (not from bugzilla), so it would be very close to the time when the patch was sent to buildbot for processing on all ews queues (except commit-queue).

Created attachment 394396 [details] Patch

(In reply to Aakash Jain from comment #4) > Comment on attachment 394376 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=394376&action=review > > >> Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:276 > >> + if patch.created < from_timestamp: > > > > Confused what this has to do with the bug title. > > Consider the scenario in which: > - a new queue 'X' was added 2 days back (with StatusBubble.DAYS_TO_CHECK = 3) > - a patch was uploaded 4 days back, and so it would never be processed on > the new queue 'X', and so the bubble for 'X' should NOT be displayed. > - Now let's say the patch was cq+ today, and so the patch-modified-time > would be updated to today. > > This would result in incorrectly showing the bubble for queue 'X' (with > position #1) instead of hiding the bubble for 'X'. This is because the logic > was based on patch-modified-time. Therefore patch created time is a better > indicator. > > Also note that the 'created' timestamp is from django database (not from > bugzilla), so it would be very close to the time when the patch was sent to > buildbot for processing on all ews queues (except commit-queue). Thanks for the explanation!

Committed r258937: <https://trac.webkit.org/changeset/258937> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394396 [details].

<rdar://problem/60839365>