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-52165] Stabilize and better test USE_WATCHING #323

Merged
merged 20 commits into from
Jul 28, 2023

Conversation

jglick
Copy link
Member

@jglick jglick commented Jul 7, 2023

JENKINS-52165: the performance benefits seem important. Have not yet tried to measure impact on (controller) CPU or Remoting channel bandwidth, but can use inotify to verify this allows a long-running sh step with copious output to only write to log (and maybe log-index), whereas in currently default pull mode there are constant writes to program.dat (via an atomic temp file) due to

} else { // legacy mode
if (controller.writeLog(workspace, listener.getLogger())) {
getContext().saveState();
which would otherwise be limited to step transitions.

Downstream of jenkinsci/workflow-api-plugin#294 and jenkinsci/durable-task-plugin#186.

@jglick

This comment was marked as resolved.

@jglick jglick requested a review from dwnusbaum July 7, 2023 19:17
jglick added 2 commits July 7, 2023 15:39
… mode since the `NewlineSafeTaskListener` was already closed by `HandlerImpl.exited`
…ShellScriptAcrossDisconnect` & `.contextualizeFreshFilePathAfterAgentReconnection`
@jglick

This comment was marked as resolved.

pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@jglick jglick requested a review from a team July 13, 2023 23:02
pom.xml Outdated Show resolved Hide resolved
@jglick

This comment was marked as resolved.

@jglick jglick changed the title [JENKINS-52165] Enable USE_WATCHING by default [JENKINS-52165] Stabilize and better test USE_WATCHING Jul 28, 2023
Copy link
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

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

I have been following this off and on in the background. I have not attempted to fully trace the reasoning behind each individual change, but I know you have tested things reasonably thoroughly, and I do not see anything that looks obviously problematic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants