Class Job needs more synchronization
------------------------------------
Key: HADOOP-3558
URL: https://issues.apache.org/jira/browse/HADOOP-3558
Project: Hadoop Core
Issue Type: Bug
Affects Versions: 0.17.0
Reporter: Aaron Greenhouse
Class Job needs additional synchronization to be truly thread-safe. At a
minimum, the following changes should be made:
* Making the field jc final.
* Adding the synchronized modifier to the methods toString(), getJobName(),
setJobName(), getJobID(), setJobID(), getMapredJobID(), setMapredJobID(),
getJobConf(), setJobConf(), getMessage(), setMessage(), getDependingJobs(),
isCompleted(), and isReady().
* Fix the method getDependingJobs() to return a *copy* of the list:
public synchronized ArrayList<Job> getDependingJobs() {
return new ArrayList<Job>(this.dependingJobs);
}
The class can be further improved, however:
* The methods setJobID() and setState() should be made package private (i.e.,
default visibility) so that they can only be called from the JobControl class.
* Reconsider the necessity of the all the getter and setter methods. In
particular, I question the need for getJobConf() and setJobConf(),
getDependingJobs(), setJobName(), setMapredJobId(), and setMesage(). If these
methods were eliminated, then the fields theJobConf and jobName could be made
final.
* The field dependingJobs could also be made final if the constructor
Job(JobConf) were changed to
public Job(JobConf jobConf) throws IOException {
this(jobConf, new ArrayList());
}
Then addDependingJob() could be simplified to
public synchronized boolean addDependingJob(Job dependingJob) {
if (this.state == Job.WAITING) { //only allowed to add jobs when waiting
return this.dependingJobs.add(dependingJob);
} else {
return false;
}
}
This class has the additional weakness that the list object referenced by field
dependingJobs is provided by the outside environment to the constructor. This
can be eliminated by copying the list when the object is constructed:
public Job(JobConf jobConf, ArrayList<Job> dependingJobs) throws IOException {
...
this.dependingJobs = new ArrayList<Job>(dependingJobs);
...
}
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
