hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steve Loughran (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HADOOP-3415) JobEndNotifier isnt synchronized, doesnt check state before acting
Date Tue, 20 May 2008 14:12:55 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-3415?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12598305#action_12598305

Steve Loughran commented on HADOOP-3415:

The JobTracker  is only a singleton with the current deployment process. There is nothing
to stop anyone imposing a different lifecycle, such as in unit tests. You can't do that now
with any isolation or thread safety aound the JobEndNotifier. Create two job trackers, you
leak one notifier thread.

1. The notifier is only live for the duration of the tracker - when jobTracker.stopTracker()
is called, the notifier is closed too. 
2. The notifier is only invoked for notifications in JobTracker, and JobInProgress, which
has its own reference to jobTracker.

With all uses via JobTracker-linked objects, and with a live lifespan of that of a JobTracker,
the obvious way to manage these instances with an instance created by the JobTracker, and
access the same way. It would only be a singleton if the JobTracker were used as a singleton
-such as when created via the command line- but not when a JobTracker is brought up differently.

> JobEndNotifier isnt synchronized, doesnt check state before acting
> ------------------------------------------------------------------
>                 Key: HADOOP-3415
>                 URL: https://issues.apache.org/jira/browse/HADOOP-3415
>             Project: Hadoop Core
>          Issue Type: Bug
>          Components: mapred
>    Affects Versions: 0.16.3
>            Reporter: Steve Loughran
>            Priority: Minor
> JobEndNotifier is pretty hazardous inside.
> 1. the static startNotifier isnt synchronized, and doesnt check for being already running
before it creates a new worker thread. It should be sycnhronized and a no-op if there is a
live thread.
> 2. stopNotifier() should be a no-op if already stopped. It MUST NOT call thread.interrupt()
in such a state, as thread may be null. 
> 3. the registerNotification method also assumes that the static queue is non null.
> Things would be a lot safer by making this class part of a JobTracker, not a singleton
with static methods, as then you could more safely make assumptions about object state. This
would not only eliminate a lot of reentrancy problems, but tie the life of the notifier to
that of its owner, the JobTracker.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message