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

Fix deadlocks on multijob plugin #65

Closed
wants to merge 16 commits into from

Conversation

Projects
None yet
3 participants
@harcher81
Copy link

harcher81 commented Mar 18, 2015

[JENKINS-27371][JENKINS-26678] Fix deadlocks on multijob plugin
[JENKINS-23967] Add support for the exception "DontTriggerException"

harcher81 added some commits Jan 22, 2015

Add call to retrieve the subjob of a subjob
Very useful with the Email-ext plugin, you can call in groovy or jelly
script this method to retrieve transitive subjob.
Add call to retrieve the subjob of a subjob
Very useful with the Email-ext plugin, you can call in groovy or jelly
script this method to retrieve transitive subjob.
[JENKINS-27371][JENKINS-26678] Fix deadlock
Fix several deadlocks, when there are many time the same job in subjob.
This greatly increases the speed of the plugin.
[JENKINS-23967] Add support for DontTriggerException
Skip the subjob when parameter throw a DontTriggerException
@jenkinsadmin

This comment has been minimized.

Copy link
Member

jenkinsadmin commented Mar 18, 2015

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

harcher81 added some commits Mar 27, 2015

Revert "Fix UT"
This reverts commit 5318545.
@@ -181,19 +184,26 @@ private boolean evaluateResult(Result result) {
private final String url;
private final boolean retry;
private final boolean aborted;
private final AbstractBuild jobBuild;

public SubBuild(String parentJobName, int parentBuildNumber,

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev May 2, 2015

Member

causes binary compatibility issues. please avoid deleting existing constructors

boolean retry, boolean aborted) {
this.parentJobName = parentJobName;
boolean retry, boolean aborted, AbstractBuild jobBuild) {
this.parentJobName = Util.fixNull(parentJobName);

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev May 2, 2015

Member

I suppose null values should be considered as bug. If not, it makes sense to add @CheckForNull attributes to the parameter and the class field

try {
return jobBuild.getLog(number);
} catch (IOException ex) {

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev May 2, 2015

Member

Bad practice. You should comment why you suppress the exception and probably write it to system logs. If the method should be public, add Javadoc as well

}

public List<SubBuild> getSubBuilds() {
if (jobBuild != null) {

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev May 2, 2015

Member

instanceof checks for null on its own

if (jobBuild instanceof MultiJobBuild) {
return ((MultiJobBuild) jobBuild).getSubBuilds();
}
} catch(ClassCastException e) {

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev May 2, 2015

Member

Overkill. It should not happen

harcher81 added some commits May 5, 2015

Add more logging
Add custom message when the job was aborted by multijob plugin

@harcher81 harcher81 closed this Aug 25, 2015

@oleg-nenashev

This comment has been minimized.

Copy link
Member

oleg-nenashev commented Aug 25, 2015

@harcher81
What was the reason of closing this PR? I see it didn't get a review from project maintainers, but there are ways to get the fix released even in such case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.