[JENKINS-16341] Automatically expire entries from BoundObjectTable
#663
Conversation
| private synchronized Ref release(String id) { | ||
| return entries.remove(id); | ||
| /** | ||
| * Releases a particular id from the table. | ||
| * Normally would be called via {@link #releaseMe} which infers the id from the current request. | ||
| */ | ||
| public void release(String id) { |
There was a problem hiding this comment.
Making this public so we can clean up the reflection in jenkinsci/jenkins@aac8c23.
| return resolve(false); | ||
| } | ||
|
|
||
| private Bound bind(Ref ref) { |
| /** | ||
| * Binds an object temporarily and returns its URL. | ||
| */ | ||
| public Bound bindWeak(Object o) { |
| */ | ||
| public static class Table implements Serializable { | ||
| private final Map<String, Ref> entries = new HashMap<>(); | ||
| private boolean logging; |
There was a problem hiding this comment.
Unnecessary, just use regular tools like https://plugins.jenkins.io/log-cli/
| // Before adding a new entry, clean up any old ones: | ||
| var it = entries.entrySet().iterator(); | ||
| while (it.hasNext()) { | ||
| var entry = it.next(); | ||
| if (!entry.getValue().valid()) { | ||
| LOGGER.fine(() -> "removing stale " + entry.getKey() + ": " | ||
| + entry.getValue().get()); | ||
| it.remove(); | ||
| } | ||
| } |
| public static boolean DEBUG_LOGGING = Boolean.getBoolean(BoundObjectTable.class.getName() + ".debugLog"); | ||
| private static final long EXPIRATION_TIME = Long.getLong( | ||
| BoundObjectTable.class.getName() + ".EXPIRATION_TIME", | ||
| Duration.ofMinutes(15).toMillis()); |
There was a problem hiding this comment.
Seems long enough for practical purposes. After this point, unused render-on-demand fragments will not work. For example, if you opened a project config form and came back 15m later and selected a parameter type from the dropdown, it will not render the per-parameter config snippet.
ProgressiveRendering, for example the agent build history screen, is unaffected since that already releases its binding as soon as it can. Similarly, used RoD fragments, such as build artifacts in jenkinsci/jenkins#10623, are released promptly anyway.
There was a problem hiding this comment.
Are we prepared to revert this based on user feedback? Because I'd be surprised if this didn't happen with some regularity:
For example, if you opened a project config form and came back 15m later and selected a parameter type from the dropdown, it will not render the per-parameter config snippet.
In particular, nontrivial freestyle or Pipeline (non-Jenkinsfile) configuration forms (with large inline scripts) could easily be open for 15 minutes while being worked on.
There was a problem hiding this comment.
If it works, sure. Or we can just increase the timeout to several hours, at which point your session is likely to have expired anyway so this would just be a fallback defense.
There was a problem hiding this comment.
Good point, cleaning up after 24 hrs or so (this approach) should not affect users while my PR keeps memory use low in the mean time, allowing for the greater timeout 👍
daniel-beck
left a comment
daniel-beck
left a comment
There was a problem hiding this comment.
Looks like a reasonable change to me. The current 1d timeout should be safe enough, and if that still accomplishes the goal, that's great.
3 participants
see JENKINS-16341
Tested with the setup in jenkinsci/jenkins#10623 (comment).