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.

Sign up for GitHub

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

Avoid saving nextBuildNumber while loading Job #9778

Merged
merged 1 commit into from
Nov 2, 2024
Merged

Avoid saving nextBuildNumber while loading Job #9778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 6 additions & 15 deletions core/src/main/java/hudson/model/Job.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,29 +221,20 @@ public void onLoad(ItemGroup<? extends Item> parent, String name)

TextFile f = getNextBuildNumberFile();
if (f.exists()) {
// starting 1.28, we store nextBuildNumber in a separate file.
// but old Hudson didn't do it, so if the file doesn't exist,
// assume that nextBuildNumber was read from config.xml
Comment on lines -224 to -226
Copy link
Member Author

Choose a reason for hiding this comment

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

A comment about Hudson 1.27 did not seem particularly helpful at this point!

try {
synchronized (this) {
this.nextBuildNumber = Integer.parseInt(f.readTrim());
}
} catch (NumberFormatException e) {
LOGGER.log(Level.WARNING, "Corruption in {0}: {1}", new Object[] {f, e});
//noinspection StatementWithEmptyBody
if (this instanceof LazyBuildMixIn.LazyLoadingJob) {
// allow LazyBuildMixIn.onLoad to fix it
Copy link
Member Author

Choose a reason for hiding this comment

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

It no longer does so, so simplify and use the same logic for all Job types. (In practice all jobs in common usage use LazyBuildMixIn.)

} else {
RunT lB = getLastBuild();
synchronized (this) {
this.nextBuildNumber = lB != null ? lB.getNumber() + 1 : 1;
}
saveNextBuildNumber();
RunT lB = getLastBuild();
synchronized (this) {
this.nextBuildNumber = lB != null ? lB.getNumber() + 1 : 1;
}
saveNextBuildNumber();
}
} else {
// From the old Hudson, or doCreateItem. Create this file now.
Copy link
Member Author

Choose a reason for hiding this comment

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

In fact doCreateItem calls onCreatedFromScratch, which did not create this file, so that was misleading.

saveNextBuildNumber();
} else if (nextBuildNumber == 0) {
nextBuildNumber = 1;
Comment on lines +236 to +237
Copy link
Member Author

@jglick jglick Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can happen if the file was absent when a job which had never been built was loaded from disk. The field initializer

jenkins/core/src/main/java/hudson/model/Job.java

Line 159 in c8bfe51

protected transient volatile int nextBuildNumber = 1;
is not called by XStream.

}

if (properties == null) // didn't exist < 1.72
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOExcep
int max = _builds.maxNumberOnDisk();
int next = asJob().getNextBuildNumber();
if (next <= max) {
LOGGER.log(Level.WARNING, "JENKINS-27530: improper nextBuildNumber {0} detected in {1} with highest build number {2}; adjusting", new Object[] {next, asJob(), max});
asJob().updateNextBuildNumber(max + 1);
LOGGER.log(Level.FINE, "nextBuildNumber {0} detected in {1} with highest build number {2}; adjusting", new Object[] {next, asJob(), max});
Copy link
Member Author

Choose a reason for hiding this comment

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

compare #2439

asJob().fastUpdateNextBuildNumber(max + 1);
Copy link
Member Author

Choose a reason for hiding this comment

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

introduced in #9019

}
RunMap<RunT> currentBuilds = this.builds;
if (parent != null) {
Expand Down
Loading