Opened 8 years ago

Closed 6 years ago

Last modified 4 years ago

#2249 closed defect (worksforme)

change merging is no longer picking the newest change

Reported by: dustin Owned by:
Priority: major Milestone: 0.8.7
Version: 0.8.6 Keywords:
Cc:

Description

See attached, where three changes were merged. They are displayed out of order on the page, and in fact the middle one numerically (4012) was the one that supplied the revision.

Attachments (1)

521.html (12.4 KB) - added by dustin 8 years ago.
521.html

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by dustin

521.html

comment:1 Changed 7 years ago by dustin

  • Milestone changed from 0.8.7 to 0.8.6

comment:2 Changed 7 years ago by dustin

  • Milestone changed from 0.8.6 to 0.8.7

I can't reproduce this externally. I'm using a sendchange loop to send changes every 2s for a 5s build, and the changes come out in the right order, and with the right revision.

Here's what I've got so far, from the db:

  • Changes 4011 and 4012 were placed in the same sourcestamp (1976) due to a tree stable timer. Change 4013 was in its own sourcestamp (1977).
  • Buildset 1980 points to ss 1976, and buildset 1981 points to ss 1977
  • os-leopard BuildRequest 42880 points to buildset 1980, and 42922 points to buildset 1981.

The build-starting process looks like this:

  • in maybeStartBuild, unclaimed_requests = [ <br 42880>, <br 42922> ] (sorted by the buildsets' submitted_at)
  • it calls _chooseBuild, which simply returns the first - <br 42880>
  • that gets merged with br 42922
  • Build() gets called with [ <br 42880>, <br 42922> ]
  • it sets its .source to <br 42880>.mergeWith([<br 42922>])
  • ..which calls SourceStamp.mergeWith
  • ..which concatenates the changes as <br 42880>.changes + <br 42922>.changes, or [ <ch 4011>, <ch 4012>, <ch 4013> ]
  • ..and calls the SourceStamp constructor, which gets the revision from the last change, and thus should get change 4013's revision.

Clearly, something did not happen as expected here: by the time SourceStamp.mergeWith got called, the build requests were in the wrong order. So either unclaimed_requests is not sorted correctly, or _chooseBuild is misbehaving, or the merging is re-ordering things.

All of those seem pretty bulletproof. unclaimed_requests is properly sorted - I tested by shuffling the list before sorting. _chooseBuild just returns [0]. And while _mergeRequests does use gatherResults, it does so in a fashion that correctly maintains order - I tested by adding a random delay to each _brdictToBuildRequest invocation.

So I'm at a loss to figure out what went wrong here. Which means I can't fix it for 0.8.6 without more evidence.

comment:3 Changed 7 years ago by dustin

  • Resolution set to worksforme
  • Status changed from new to closed

Let's re-open this if we find more data..

comment:4 Changed 6 years ago by brendan

I've been struck by this issue. In my case, the sourcestamps have no associated change lists (not sure why, but probably because the builder was scheduled by a trigger -- though the invoker's sourcestamps have single entry change lists). Looking at sourceStamp.mergeWith, it seems you'd get the oldest revision if the changes list is empty?

comment:5 Changed 6 years ago by brendan

  • Resolution worksforme deleted
  • Status changed from closed to reopened

comment:6 Changed 6 years ago by dustin

Based on BuildRequest.mergeSourceStampsWith, in the absence of changes you'll get the sourcestamp of the first buildrequest handed to the Build constructor, which based on my notes in comment 2 should give you the first one sorted by submitted_at. Is that not what you're seeing?

Given the mystery above, we need more than "saw this too" to track this one down, unfortunately.

comment:7 follow-up: Changed 6 years ago by brendan

  • Resolution set to worksforme
  • Status changed from reopened to closed

Moving my version of this bug to a separate ticket strictly for no-changes merges.

comment:8 in reply to: ↑ 7 Changed 4 years ago by dcoshea

Replying to brendan:

Moving my version of this bug to a separate ticket strictly for no-changes merges.

This is #2680.

Note: See TracTickets for help on using tickets.