500194 - Graph server reports bogus changeset ids

Status: RESOLVED FIXED

(Webtools Graveyard :: Graph Server, defect)

Description

[Boris Zbarsky [:bzbarsky]]

STEPS TO REPRODUCE:
1)  Load http://graphs-new.mozilla.org/graph.html#tests=[{%22test%22:%2218%22,%22branch%22:%221%22,%22machine%22:%22132%22}]&sel=1245710580,1245883380

2)  Hover over the point on the graph that's at about 6PM on June 23 2009 on the X axis and about at 765 on the Y axis.

3)  See what the blue text at the top of the graph says:

ACTUAL RESULTS:
"Date: 6/23/2009 6:08 PM Value: 765.15 Changeset: 34253c5fa8b7"

EXPECTED RESULTS: List the correct changeset id.  In this case, the build in question is this one, according to tinderbox:

  s: talos-rev2-vista05
  id:20090623180800
  rev:7df73c97307a

At least if I assume the actual value of 765.15 is correct.

Comment 1

[Ryan Doherty (:rdoherty)]

I'm not able to reproduce this, if I hover over the date specified, I see the correct changeset. Will attach screenshot.

Comment 2

[Ryan Doherty (:rdoherty)]

Attachment: screenshot

Comment 3

[Ryan Doherty (:rdoherty)]

Boris, are you running nightlies? Can I get a screenshot of your behavior and any other details?

Comment 4

[Ryan Doherty (:rdoherty)]

Ok, Chris helped me find a test case where this occurs.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Boris, are you running nightlies?

Yes.  And today I see what your screenshot shows and NOT what I saw when I filed the bug.

I'm glad you figured out a way to reproduce.

Comment 7

[Chris AtLee [:catlee]]

Any progress on this?

Comment 8

[Ryan Doherty (:rdoherty)]

(In reply to comment #7)
> Any progress on this?

Not really, swamped with other things, won't be able to get to this for about a week.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Any progress on this?

Comment 11

[Ryan Doherty (:rdoherty)]

(In reply to comment #10)
> Any progress on this?

Nope, 3.6 work has taken priority.

Comment 12

[Chris AtLee [:catlee]]

This makes regression tracking very difficult.  What's the ETA on this?

Comment 13

[Ryan Doherty (:rdoherty)]

(In reply to comment #12)
> This makes regression tracking very difficult.  What's the ETA on this?

working on it now, hoping to have a fix today.

Comment 14

[Ryan Doherty (:rdoherty)]

Still working on this, not confident anymore on finding a fix soon, extremely esoteric bug. If someone is feeling daring, lmk and I can point you in the direction of where I think the bug is.

Comment 16

[Shawn Wilsher :sdwilsh]

So, this is like 100% reproducible for me and makes finding a regression based on the graph next to impossible (I have to resort to build times, which I don't exactly trust, and have to cross reference with logs).

Comment 17

[Chris AtLee [:catlee]]

Any progress here?  This makes regression hunting very difficult.

Comment 18

[andershol]

Attachment: Initialize extra_info

The type "TimeValueDataSet", declared in "js/DataSet.js", is used to hold the datasets. Changeset info is kept in the "extra_data" array field separate from the "data" array field that holds the x/y values for the plots. But the "extra_data" array instance is only initialized in the prototype, so it is reused for all "TimeValueDataSet" instances. This means that when a dataset is loaded, it will overwrite the changesets of any previously loaded dataset (or the begining of it, if the new has fewer values).

Not sure if this solves all the observed problems, since the posted examples only had a single graph. But for the graph:
http://graphs.mozilla.org/graph.html#tests=[[16,1,354],[16,1,357],[16,1,358],[16,1,359]]&sel=1275415106,1275861378
Hovering the test "Ts on WINNT 6.1, talos-r3-w7-007, Firefox branch" at "Date: 6/6/2010 4:52 AM Value: 412.26" shows "Changeset: 4a4e6dea49bd" before and "Changeset: ac1ed3f6b2e7" after, which seems correct if I am reading this correctly:
http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox&maxdate=1275861378&legend=0&norules=1

Comment 19

[Karl Tomlinson (:karlt)]

Comment on attachment 449834 [details] [diff] [review]
Initialize extra_info

Thanks so much more looking at this.
It would be fantastic to be able to use these graphs again.
Who needs to review this?

Comment 20

[Shawn Wilsher :sdwilsh]

(In reply to comment #18)
> Not sure if this solves all the observed problems, since the posted examples
> only had a single graph. But for the graph:
> http://graphs.mozilla.org/graph.html#tests=[[16,1,354],[16,1,357],[16,1,358],[16,1,359]]&sel=1275415106,1275861378
> Hovering the test "Ts on WINNT 6.1, talos-r3-w7-007, Firefox branch" at "Date:
> 6/6/2010 4:52 AM Value: 412.26" shows "Changeset: 4a4e6dea49bd" before and
> "Changeset: ac1ed3f6b2e7" after, which seems correct if I am reading this
> correctly:
> http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox&maxdate=1275861378&legend=0&norules=1
You are awesome.  No, really really awesome.

Comment 21

[Mike Shaver (:shaver emeritus)]

This is fantastic!  You're a hero.

Comment 22

[Ryan Doherty (:rdoherty)]

Comment on attachment 449834 [details] [diff] [review]
Initialize extra_info

omg seriously that's it? Looks good to me, commit it, we'll see if it works on stage.

Comment 23

[andershol]

(In reply to comment #22)
> omg seriously that's it?
Not sure, but it is a step. Do you still have the test case from comment #4?

> Looks good to me, commit it, we'll see if it works on stage.
I believe, I don't have commit access. First patch.

Comment 24

[Ryan Doherty (:rdoherty)]

(In reply to comment #23)
> (In reply to comment #22)
> > omg seriously that's it?
> Not sure, but it is a step. Do you still have the test case from comment #4?

Test case iirc was loading one chart, find a date+revision pair, load another and find the same date, check that the revision is different. 

> > Looks good to me, commit it, we'll see if it works on stage.
> I believe, I don't have commit access. First patch.

http://hg.mozilla.org/graphs/rev/c60e806e4964

Comment 25

[Ryan Doherty (:rdoherty)]

Looks good on stage to me, closing.