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 broken caching handling of job renames by purging cache entry [JENKINS-38159] #43

Merged
merged 1 commit into from May 30, 2017

Conversation

svanoort
Copy link
Member

JENKINS-38159 - log URLs would have to be rewritten (as well as all HAL links in JSON), so it's better to just purge the cache entry and rebuild when needed.

…work cache invalidation for efficiency [JENKINS-38159]
@svanoort
Copy link
Member Author

@reviewbybees

@ghost
Copy link

ghost commented May 30, 2017

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

}
}
ConcurrentMap<String, RunExt> runMap = rc.asMap();
for (String cacheRunId : runMap.keySet()) { // Put the Concurrent in ConcurrentMap to work for us
Copy link
Member

Choose a reason for hiding this comment

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

Maybe better style to use Iterator.remove.

}
ConcurrentMap<String, RunExt> runMap = rc.asMap();
for (String cacheRunId : runMap.keySet()) { // Put the Concurrent in ConcurrentMap to work for us
// Null-check may not be needed, but just in case of mutation by another thread
Copy link
Member

Choose a reason for hiding this comment

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

Would that really result in null entries??

Copy link
Member Author

Choose a reason for hiding this comment

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

@jglick I hope not, but am feeling a bit cautious - plus it's easier to put the check than to prove it safe.

@svanoort svanoort merged commit 6a302eb into master May 30, 2017
@svanoort svanoort deleted the fix-caching-with-renames-JENKINS-38159 branch May 30, 2017 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants