Closed Bug 621456 Opened 14 years ago Closed 14 years ago

Showing 10 pushes on the front page is wasteful when they're old

Categories

(Tree Management Graveyard :: TBPL, defect)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philor, Assigned: mstange)

References

Details

Attachments

(2 files)

Right now, I'm sitting on the tip of TraceMonkey, where 10 pushes takes us back three days to Thursday morning. Every refresh, I'm repeatedly requesting the 12 hour span before 20:31:17 GMT Thursday as though something will change that long ago, and I'll continue to download that over my capped 3G connection (since Tinderbox isn't smart enough to return a 304 for something which hasn't changed for two days, and never again will change), until someone pushes and drops that last one off the bottom end. While we always want to show the tip rev, if the things before it are more than 24 hours old, we should just drop them for an initial load.
(In reply to comment #0) > Every refresh, I'm repeatedly requesting the 12 > hour span before 20:31:17 GMT Thursday as though something will change that > long ago That sounds like the main bug here. > While we always want to show the tip rev, if the things before it are more than > 24 hours old, we should just drop them for an initial load. We can’t do that currently. There is no way to tell pushlog to load “all the newest pushes in the last 24 hours, but at most 10 of them”
Yes. The bug is that just by having http://tbpl.mozilla.org/?tree=TraceMonkey open just now, I was making 280 completely pointless requests an hour to the creaking Tinderbox cgi (until I reverted the bug 619673 stuff on my own instance and switched to using it). The mozilla-central tree is only making 30 completely pointless requests an hour, but if 20 people have it open that's still 600 pointless requests an hour, and the way I read it, if they've left it open since this morning, it's probably more like 1200 or 1800 an hour, depending on how old the Sunday pushes were. I wonder why Tinderbox has been so slow last week and over the weekend, quite often needing multiple clicks on a failed test run to get tbpl to actually manage to load the log?
Severity: enhancement → critical
This is probably causing (or contributing to the cause of) bug 623488.
Have you entered the correct bug number or is tbpl really causing a security relevant bug with access restriction?
(In reply to comment #4) > Have you entered the correct bug number or is tbpl really causing a security > relevant bug with access restriction? It's an IT bug currently titled "tinderbox.mozilla.org is overloaded and down", and, yes, it is the right number. (I don't see anything in it worthy of keeping it hidden, but I'm also not going to be the person to act upon that determination.) Shawn posted a log of requests made loading the Places tbpl, and because the branch doesn't have ten pushes particularly recently, the total request count made is ~70 (!), about 40 of which are http://tinderbox.mozilla.org/showbuilds.cgi?tree=Places&json=1&maxdate=1293123563&hours=12 and similar URLs and not just constant tbpl resources.
This fixes probably the most pressing issue: (In reply to comment #0) > where 10 pushes takes us back > three days to Thursday morning. Every refresh, I'm repeatedly requesting the 12 > hour span before 20:31:17 GMT Thursday
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Attachment #504539 - Flags: review?(arpad.borsos)
This makes sure that at least one push is shown by measuring the 24 hours relatively to the latest push, and not to the current time.
Attachment #504546 - Flags: review?(arpad.borsos)
Comment on attachment 504539 [details] [diff] [review] part 1: don't request old data from Tinderbox Wouldn’t it be easier to just save the start/endtime that we have loaded, similar to how we load the pushes instead of saving the latestTime for each push? >+ _getRequestStartForPushes: function TinderboxJSONUser__getRequestStartForPushes(pushes, now) { >+ // Return the earliest time for which there's no existing Tinderbox data >+ // for any push in pushes. >+ return pushes.reduce(function (startTime, push) { >+ return Math.min(startTime, push.latestTinderboxData || push.date); >+ }, now.getTime()); >+ }, Isn’t Array.reduce() a SpiderMonkey extension?
Attachment #504539 - Flags: review?(arpad.borsos) → review+
Comment on attachment 504546 [details] [diff] [review] part 2: restrict the default view push range to 24 hours > if ("startdate" in pushRange && "enddate" in pushRange) > return [{ startdate: pushRange.startdate, enddate: pushRange.enddate }]; >+ if ("maxhours" in pushRange) >+ return [{ maxhours: pushRange.maxhours }]; > return [{}]; Please add a comment that this parameter does not directly map to pushlog parameters but is just used by tbpl internally.
Attachment #504546 - Flags: review?(arpad.borsos) → review+
(In reply to comment #8) > Comment on attachment 504539 [details] [diff] [review] > part 1: don't request old data from Tinderbox > > Wouldn’t it be easier to just save the start/endtime that we have loaded, > similar to how we load the pushes instead of saving the latestTime for each > push? Probably, but I don't want to pull too much of this into Data because I want to keep Data backend-neutral. Time ranges for Tinderbox data only map correctly to the Tinderbox backend, not to a per-revision Buildbot backend. > Isn’t Array.reduce() a SpiderMonkey extension? MDC says "ECMA-262 Edition 5" (whatever that means) and it's supported in the Opera and Webkit builds that I have lying around here. (In reply to comment #9) > Please add a comment that this parameter does not directly map to pushlog > parameters but is just used by tbpl internally. Good idea.
Product: Webtools → Tree Management
Product: Tree Management → Tree Management Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: