[
https://issues.apache.org/jira/browse/HADOOP-3558?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Aaron Greenhouse updated HADOOP-3558:
-------------------------------------
Attachment: Job.patch
Patch file for applying the above changes to the class.
> 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
> Attachments: Job.patch
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> 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.
