Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(concourse): Fix caching of concourse build events #525

Merged
merged 2 commits into from
Oct 23, 2019

Conversation

donnyyung
Copy link
Contributor

Builds should be added to cache correctly now and the admin fastforward command should work
It solves spinnaker/spinnaker#5047

I ran into this problem because we had over 1000 builds on our concourse instance, so we were running into cache exceeds upper threshold (1000).

Upon trying to fix this, I realized the builds were never being compared to what was in the cache until the commitDelta function, which unfortunately we don't get to unless the threshold is less than 1000.

Note: This is my first PR to a spinnaker project and my first time coding in java so open to any and all suggestions!

@@ -124,8 +124,13 @@ private JobDelta jobDelta(ConcourseProperties.Host host, Job job) {
List<GenericBuild> genericBuilds =
builds.stream()
.map(build -> concourseService.getGenericBuild(jobPath, build, false))
.filter(b -> !cache.getEventPosted(host, job, cursor, b.getNumber()))
Copy link
Contributor

Choose a reason for hiding this comment

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

We still want to run cache.setLastPollCycleTimestamp() for jobs without uncommitted successful builds, which filtering here prevents.

Copy link
Contributor Author

@donnyyung donnyyung Oct 22, 2019

Choose a reason for hiding this comment

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

Ah okay. I should be able to add a cache.setLastPollCycleTimestamp() in the if right after this then right?

I believe we need the filter + return null there when there are no uncommitted successful builds or else the JobPollingDelta list will always be the # of concourse jobs, correct?
Just want to make sure I'm understanding the problem.

Nvrmind that seems to make me get stuck in a loop where it continually is pushing and caching everything over and over again.. Will take a look and figure out correct way to fix this.


Figured it out.

@donnyyung donnyyung force-pushed the fix_concourse_fastforward branch 2 times, most recently from c1f9197 to 1c77330 Compare October 23, 2019 14:42
Builds should be added to cache correctly now and the admin fastforward command should work
spinnaker/spinnaker#5047
@asher asher self-requested a review October 23, 2019 18:47
@asher asher added the ready to merge Approved and ready for merge label Oct 23, 2019
@mergify mergify bot merged commit 9430260 into spinnaker:master Oct 23, 2019
@mergify mergify bot added the auto merged label Oct 23, 2019
@donnyyung
Copy link
Contributor Author

@spinnakerbot cherry-pick 1.16

spinnakerbot pushed a commit that referenced this pull request Oct 23, 2019
Builds should be added to cache correctly now and the admin fastforward command should work
spinnaker/spinnaker#5047
@spinnakerbot
Copy link
Contributor

Cherry pick successful: #530

louisjimenez pushed a commit that referenced this pull request Oct 23, 2019
Builds should be added to cache correctly now and the admin fastforward command should work
spinnaker/spinnaker#5047
Shimiazoulai pushed a commit to spotinst/spinnaker-igor that referenced this pull request Jan 26, 2023
…spinnaker#530)

Builds should be added to cache correctly now and the admin fastforward command should work
spinnaker/spinnaker#5047
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged ready to merge Approved and ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants