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

[JENKINS-60625] Prevent Jenkins page rendering from being blocked when the update center data parsing is in progress #4413

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 31 additions & 0 deletions core/src/main/java/hudson/model/UpdateCenter.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import jenkins.util.Timer;
import org.apache.commons.io.IOUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -207,6 +208,9 @@ public class UpdateCenter extends AbstractModelObject implements Saveable, OnMas

private boolean requiresRestart;

/** @see #isSiteDataReady */
private transient volatile boolean siteDataLoading;

static {
Logger logger = Logger.getLogger(UpdateCenter.class.getName());
LOGGER = logger;
Expand Down Expand Up @@ -539,6 +543,27 @@ public PersistedList<UpdateSite> getSites() {
return sites;
}

/**
* Whether it is <em>probably</em> safe to call all {@link UpdateSite#getData} without blocking.
* @return true if all data is <em>currently</em> ready (or absent);
* false if some is not ready now (but it will be loaded in the background)
*/
@Restricted(NoExternalUse.class)
public boolean isSiteDataReady() {
if (sites.stream().anyMatch(UpdateSite::hasUnparsedData)) {
if (!siteDataLoading) {
siteDataLoading = true;
Timer.get().submit(() -> {
sites.forEach(UpdateSite::getData);
siteDataLoading = false;
});
}
return false;
} else {
return true;
}
}

/**
* The same as {@link #getSites()} but for REST API.
*/
Expand Down Expand Up @@ -927,6 +952,7 @@ public synchronized void load() throws IOException {
sites.add(createDefaultUpdateSite());
}
}
siteDataLoading = false;
}

protected UpdateSite createDefaultUpdateSite() {
Expand Down Expand Up @@ -1061,7 +1087,12 @@ public String getDisplayName() {
return Messages.UpdateCenter_CoreUpdateMonitor_DisplayName();
}

@Override
public boolean isActivated() {
if (!Jenkins.get().getUpdateCenter().isSiteDataReady()) {
Copy link
Member

Choose a reason for hiding this comment

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

Note to self: I missed this change in the original version of the previous review.

// Do not display monitor during this page load, but possibly later.
return false;
}
Data data = getData();
return data!=null && data.hasCoreUpdates();
}
Expand Down
13 changes: 12 additions & 1 deletion core/src/main/java/hudson/model/UpdateSite.java
Original file line number Diff line number Diff line change
Expand Up @@ -315,14 +315,25 @@ public Data getData() {
return data;
}

/**
* Whether {@link #getData} might be blocking.
*/
// Internal use only
boolean hasUnparsedData() {
jglick marked this conversation as resolved.
Show resolved Hide resolved
return data == null && getDataFile().exists();
}

/**
* Gets the raw update center JSON data.
*/
public JSONObject getJSONObject() {
TextFile df = getDataFile();
if(df.exists()) {
long start = System.nanoTime();
try {
return JSONObject.fromObject(df.read());
JSONObject o = JSONObject.fromObject(df.read());
LOGGER.fine(() -> String.format("Loaded and parsed %s in %.01fs", df, (System.nanoTime() - start) / 1_000_000_000.0));
return o;
} catch (JSONException | IOException e) {
LOGGER.log(Level.SEVERE,"Failed to parse "+df,e);
df.delete(); // if we keep this file, it will cause repeated failures
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@
public class UpdateSiteWarningsMonitor extends AdministrativeMonitor {
@Override
public boolean isActivated() {
if (!Jenkins.get().getUpdateCenter().isSiteDataReady()) {
return false;
}
return !getActiveCoreWarnings().isEmpty() || !getActivePluginWarningsByPlugin().isEmpty();
}

Expand Down