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

Jump to bottom

[FIXED JENKINS-34213] Ensure that the unexporter cleans up whatever it can each sweep #81

Merged
merged 6 commits into from
Apr 20, 2016
Merged

[FIXED JENKINS-34213] Ensure that the unexporter cleans up whatever it can each sweep #81

merged 6 commits into from
Apr 20, 2016

Conversation

stephenc
Copy link
Member

@reviewbybees

@ghost
Copy link

ghost commented Apr 13, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

oleg-nenashev
oleg-nenashev reviewed Apr 13, 2016
View reviewed changes
src/main/java/hudson/remoting/RemoteInvocationHandler.java
* you can reduce GC pressure by discarding the origin call-site stack traces and setting this system
* property.
*/
private static final boolean retainOrigin =
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, but it seems to be a useful addition

@stephenc
Copy link
Member Author

FYI my stress testing of this fell over into Full GCs... I may need to refine further

@jenkinsadmin
Copy link
Member

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

@stephenc
[JENKINS-34213] Collect and report meaningful stats, batch the refere…
98a7e3a 
…nce collection for better stability

- We want to report on these things only if they are an issue. Logging of the actual stats should be below the radar of the users using a default logger level of `INFO` provided that the Unexporter is not doing much
- When the Unexporter is busy (i.e. the m1 rate is > 100/sec) then we should start reporting at `INFO`
- In the event that there is sustained high levels of work, we should alert the user and recommend turning off the stack traces to reduce GC pressure

- My stress testing revealed that under very heavy load it is better to batch the removal and then batch the clean-up even if this batching means that the sweeps are not as frequent.
@stephenc
Copy link
Member Author

@oleg-nenashev should be ready now! I'm not seeing this one fall over into full GCs... of course will leave it running some more ;-)

@jtnord
Copy link
Member

jtnord commented Apr 15, 2016

🐝

2 similar comments
@andresrc
Copy link

🐝

@aheritier
Copy link
Member

🐝

@stephenc
Copy link
Member Author

@reviewbybees done

@stephenc
Copy link
Member Author

@jenkins/code-reviewers

@stephenc
Copy link
Member Author

@jenkinsci/code-reviewers

@ghost
Copy link

ghost commented Apr 15, 2016

This pull request has completed our internal processes and we now respectfully request the maintainers of this repository to consider our proposal contained within this pull request for merging.

@oleg-nenashev
Copy link
Member

Ready to be integrated.
We will likely need to backport this fix, so please squash the commits.

@oleg-nenashev oleg-nenashev merged commit f4d2876 into master Apr 20, 2016
oleg-nenashev added a commit to oleg-nenashev/jenkins that referenced this pull request May 13, 2016
@oleg-nenashev
[JENKINS-19445, JENKINS-34213, JENKINS-34808] Bump remoting to 2.58.
ff0bae2 
Changes:
* [JENKINS-34213](https://issues.jenkins-ci.org/browse/JENKINS-34213) - Ensure that the unexporter cleans up whatever it can each sweep (jenkinsci/remoting#81)
* [JENKINS-19445](https://issues.jenkins-ci.org/browse/JENKINS-19445) Force class load on UserRequest in order to prevent deadlock on windows nodes when using JNA and Subversion (jenkinsci/remoting#81)
* [JENKINS-34808](https://issues.jenkins-ci.org/browse/JENKINS-34808) - Allow user to adjust socket timeout (jenkinsci/remoting#68)
@oleg-nenashev oleg-nenashev mentioned this pull request May 13, 2016
Merged
oleg-nenashev added a commit to jenkinsci/jenkins that referenced this pull request May 14, 2016
@oleg-nenashev
[JENKINS-19445, JENKINS-34213, JENKINS-34808, JENKINS-34121] Bump rem…
409438f 
…oting to 2.59. (#2344)

* [JENKINS-19445, JENKINS-34213, JENKINS-34808] Bump remoting to 2.58.

Changes:
* [JENKINS-34213](https://issues.jenkins-ci.org/browse/JENKINS-34213) - Ensure that the unexporter cleans up whatever it can each sweep (jenkinsci/remoting#81)
* [JENKINS-19445](https://issues.jenkins-ci.org/browse/JENKINS-19445) Force class load on UserRequest in order to prevent deadlock on windows nodes when using JNA and Subversion (jenkinsci/remoting#81)
* [JENKINS-34808](https://issues.jenkins-ci.org/browse/JENKINS-34808) - Allow user to adjust socket timeout (jenkinsci/remoting#68)

* [JENKINS-34121] - Upgrade remoting to 2.59
oleg-nenashev added a commit to oleg-nenashev/jenkins that referenced this pull request May 17, 2016
@oleg-nenashev
[JENKINS-19445, JENKINS-34213, JENKINS-34808, JENKINS-34121] Bump rem…
09e482f 
…oting to 2.59

Changes:

2.58:
* [JENKINS-34213](https://issues.jenkins-ci.org/browse/JENKINS-34213) - Ensure that the unexporter cleans up whatever it can each sweep (jenkinsci/remoting#81)
* [JENKINS-19445](https://issues.jenkins-ci.org/browse/JENKINS-19445) Force class load on UserRequest in order to prevent deadlock on windows nodes when using JNA and Subversion (jenkinsci/remoting#82)
* [JENKINS-34808](https://issues.jenkins-ci.org/browse/JENKINS-34808) - Allow user to adjust socket timeout (jenkinsci/remoting#68)

2.59:
* [JENKINS-34819](https://issues.jenkins-ci.org/browse/JENKINS-34819) - Allow disabling the remoting protocols individually. Works around issues like [JENKINS-34121](https://issues.jenkins-ci.org/browse/JENKINS-34121) (jenkinsci/remoting#83)
@oleg-nenashev oleg-nenashev mentioned this pull request May 17, 2016
Closed
olivergondza pushed a commit to jenkinsci/jenkins that referenced this pull request May 25, 2016
@oleg-nenashev @olivergondza
[JENKINS-19445, JENKINS-34213, JENKINS-34808, JENKINS-34121] Bump rem…
12e7996 
…oting to 2.59. (#2344)

* [JENKINS-19445, JENKINS-34213, JENKINS-34808] Bump remoting to 2.58.

Changes:
* [JENKINS-34213](https://issues.jenkins-ci.org/browse/JENKINS-34213) - Ensure that the unexporter cleans up whatever it can each sweep (jenkinsci/remoting#81)
* [JENKINS-19445](https://issues.jenkins-ci.org/browse/JENKINS-19445) Force class load on UserRequest in order to prevent deadlock on windows nodes when using JNA and Subversion (jenkinsci/remoting#81)
* [JENKINS-34808](https://issues.jenkins-ci.org/browse/JENKINS-34808) - Allow user to adjust socket timeout (jenkinsci/remoting#68)

* [JENKINS-34121] - Upgrade remoting to 2.59

(cherry picked from commit 409438f)
@stephenc stephenc deleted the jenkins-34213 branch July 28, 2016 08:01
@jglick jglick mentioned this pull request Aug 20, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
backporting-candidate ready-to-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@stephenc @jenkinsadmin @jtnord @andresrc @aheritier @oleg-nenashev