Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

make image sort stable, and correct #1247

Merged
merged 1 commit into from
Jul 24, 2018
Merged

make image sort stable, and correct #1247

merged 1 commit into from
Jul 24, 2018

Conversation

rade
Copy link
Contributor

@rade rade commented Jul 24, 2018

The ByCreatedDesc sort was broken in two ways:

  1. If there were two images with a zero CreatedAt time, the sort order would depend on the order in which they appeared in the orginal (unsorted) list, thus making the order unstable.

  2. Images with a zero CreatedAt time were considered more recent than any images with a non-zero CreatedAt time, which is the exact opposite of what we want.

These bugs may be the cause of the 'flipping' between different images we are seeing in #1127.

@rade rade requested a review from squaremo July 24, 2018 12:12
The ByCreatedDesc sort was broken in two ways:

1. If there were two images with a zero CreatedAt time, the sort order
would depend on the order in which they appeared in the
orginal (unsorted) list, thus making the order unstable.

2. Images with a zero CreatedAt time were considered more recent than
any images with a non-zero CreatedAt time, which is the exact opposite
of what we want.

These bugs _may_ be the cause of the 'flipping' between different
images we are seeing in #1127.
@rade rade force-pushed the stable-image-sort branch from d3ceea6 to aca71d8 Compare July 24, 2018 12:14
Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks Matthias 🎇

checkSorted(t, imgs)
}

func checkSorted(t *testing.T, imgs []Info) {

This comment was marked as abuse.

This comment was marked as abuse.

@rade rade merged commit 5bd3929 into master Jul 24, 2018
rade added a commit that referenced this pull request Jul 24, 2018
An error while fetching an image manifest would return a nil
error (hence indicating success) with a unit value image.Info{}
struct.

That is bad news for the caller in Warmer.warm(), which will map an
image tag to that empty image.Info{}, polluting the cache entry for
the image+tag and image in memcached.

When we subsequently use this info to determine the latest suitable
tag, we encounter zero CreatedAt timestamps, which, prior to the
changes in #1247, #1249 and #1250 would cause the wrong images to be
released.

Fixes #1127.
@hiddeco hiddeco deleted the stable-image-sort branch April 11, 2019 13:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants