[JENKINS-70531] Apply timeout on WebSocket write operations #7596
Conversation
| // As in Engine.runWebSocket: | ||
| LOGGER.finest(() -> "sending message of length " + (headerAndData.remaining() - 2)); | ||
| try { | ||
| sendBinary(headerAndData).get(5, TimeUnit.MINUTES); |
There was a problem hiding this comment.
Does this mean that any channel write operation should be under 5 minutes?
I know it's a bad practice, but what if a plugin is copying big files from the agent to the controller, that could easily take more than 5 minutes.
There was a problem hiding this comment.
You mean if you are using archiveArtifacts without artifact-manager-s3. Yes that could easily run over 5m for one copy operation; but this timeout is for one transport frame, which is currently only 8k (for WebSocket it could probably be increased considerably). Even one Command (broken into frames) would not necessarily be the full file size, since you would be using a RemoteOutputStream sending ProxyOutputStream.Chunks which default to 1Mb. (all IIUC, but certainly worth testing)
jglick
changed the title
|
@jglick i know this is still in draft, but I was wondering, would this PR be considered for lts-backport? |
|
Perhaps. I added |
MarkEWaite
added
bug
|
This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. |
…i#7596) * Apply timeout on WebSocket write operations * jenkinsci/remoting#621 released (cherry picked from commit e0aee59)
Matches jenkinsci/remoting#621. https://issues.jenkins.io/browse/JENKINS-70531
Testing done
See upstream.
Proposed changelog entries
Maintainer checklist
Before the changes are marked as
ready-for-merge:upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered (see query).