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

Retry for transient exceptions while doing cleanup for Hadoop Jobs #3177

Merged
merged 3 commits into from
Jun 23, 2016
Merged

Retry for transient exceptions while doing cleanup for Hadoop Jobs #3177

merged 3 commits into from
Jun 23, 2016

Conversation

nishantmonu51
Copy link
Member

fixes #1828

@fjy
Copy link
Contributor

fjy commented Jun 22, 2016

👍

@fjy fjy added the Bug label Jun 22, 2016
@fjy fjy added this to the 0.9.2 milestone Jun 22, 2016
@nishantmonu51
Copy link
Member Author

travis failure with AnnouncerTest - https://travis-ci.org/druid-io/druid/jobs/139633994

@nishantmonu51 nishantmonu51 reopened this Jun 23, 2016
drcrallen
drcrallen reviewed Jun 23, 2016
View reviewed changes
indexing-hadoop/src/main/java/io/druid/indexer/JobHelper.java
@@ -44,6 +44,7 @@
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.retry.RetryPolicies;
import org.apache.hadoop.io.retry.RetryProxy;
import org.apache.hadoop.ipc.Server;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused import?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

@drcrallen
Copy link
Contributor

@nishantmonu51 can you make the title describe what the patch does?

Also is there any test which would be appropriate to add for this?

@fjy
Copy link
Contributor

fjy commented Jun 23, 2016

@drcrallen I think difficult to test

@nishantmonu51 nishantmonu51 changed the title fix 1828 Retry for transient exceptions while doing cleanup for Hadoop Jobs Jun 23, 2016
drcrallen
drcrallen reviewed Jun 23, 2016
View reviewed changes
indexing-hadoop/src/main/java/io/druid/indexer/updater/HadoopConverterJob.java
fs.delete(jobDir, true);
fs.delete(getJobClassPathDir(job.getJobName(), job.getWorkingDirectory()), true);
JobHelper.deleteWithRetry(fs, jobDir, true);
JobHelper.deleteWithRetry(fs, getJobClassPathDir(job.getJobName(), job.getWorkingDirectory()), true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this fire regardless of if the prior delete succeeds?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, fixed.

@drcrallen
Copy link
Contributor

👍 after travis

@drcrallen drcrallen merged commit 2696b0c into apache:master Jun 23, 2016
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
Bug
Projects
None yet
Milestone
0.9.2
Development

Successfully merging this pull request may close these issues.

Retry for transient exceptions while doing cleanup for Hadoop Jobs
3 participants
@nishantmonu51 @fjy @drcrallen