Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Duplicate
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      Add support to AbstractService that allow callers to register listeners for all instances. The existing listener interface could be used. This allows management tools to hook into the events.

      The static listeners would be invoked for all state changes except creation (base class shouldn't be handing out references to itself at this point).

      These static events could all be async, pushed through a shared ConcurrentLinkedQueue; failures logged at warn and the rest of the listeners invoked.

      1. MAPREDUCE-3995.patch
        3 kB
        Steve Loughran
      2. MAPREDUCE-3995.patch
        2 kB
        Steve Loughran
      3. MAPREDUCE-3995.patch
        2 kB
        Steve Loughran
      4. MAPREDUCE-3995.patch
        15 kB
        Steve Loughran
      5. MAPREDUCE-3995.patch
        17 kB
        Steve Loughran
      6. MAPREDUCE-3995.patch
        17 kB
        Steve Loughran

        Issue Links

          Activity

          Hide
          Steve Loughran added a comment -

          First pass for review -no tests.

          This is designed to be as simple as possible -the same service listener is used for the "global" listeners, and the notifications are blocking on state changes. I started to write something asynchronous but it was over-complex, hard to test and could easily leak threads.

          The global listener list is synchronised on the list itself, both for changes and for the list run through itself. Listeners must not attempt to unregister themselves during a callback or the iterator will fail.

          Show
          Steve Loughran added a comment - First pass for review -no tests. This is designed to be as simple as possible -the same service listener is used for the "global" listeners, and the notifications are blocking on state changes. I started to write something asynchronous but it was over-complex, hard to test and could easily leak threads. The global listener list is synchronised on the list itself, both for changes and for the list run through itself. Listeners must not attempt to unregister themselves during a callback or the iterator will fail.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12517753/MAPREDUCE-3995.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2027//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12517753/MAPREDUCE-3995.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2027//console This message is automatically generated.
          Hide
          Steve Loughran added a comment -

          proper diff

          Show
          Steve Loughran added a comment - proper diff
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12517784/MAPREDUCE-3995.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in .

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2029//testReport/
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2029//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12517784/MAPREDUCE-3995.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2029//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2029//console This message is automatically generated.
          Hide
          Steve Loughran added a comment -

          patch off while the next stage goes in:

          1. useful listener -one that logs.
          2. test listener -one that caches event history and can be set to fail on a specific state event; put this in test/ .
          Show
          Steve Loughran added a comment - patch off while the next stage goes in: useful listener -one that logs. test listener -one that caches event history and can be set to fail on a specific state event; put this in test/ .
          Hide
          Steve Loughran added a comment -
          1. unregistering a global listener returns true iff the listener was registered. Needed for testing.
          1. adds one useful listener that logs the events via commons logging.
          1. adds a listener designed to count the number of events received, cache the last one, and can be set to fail on any specific state change event.
          1. add tests to verify listener register/unregister, event callback, and explore the behaviour of listeners when listeners themselves fail.
          Show
          Steve Loughran added a comment - unregistering a global listener returns true iff the listener was registered. Needed for testing. adds one useful listener that logs the events via commons logging. adds a listener designed to count the number of events received, cache the last one, and can be set to fail on any specific state change event. add tests to verify listener register/unregister, event callback, and explore the behaviour of listeners when listeners themselves fail.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12517859/MAPREDUCE-3995.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in .

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2033//testReport/
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2033//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12517859/MAPREDUCE-3995.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2033//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2033//console This message is automatically generated.
          Hide
          Steve Loughran added a comment -

          latest diff

          Show
          Steve Loughran added a comment - latest diff
          Hide
          Steve Loughran added a comment -

          Patch with test. I'm submitting to see what Jenkins says, but I've realised that adding the listeners to AbstractService isn't the right place to add a list of static listeners.

          It would be better to move the code to the ServiceOperations class of MAPREDUCE-3970, and then invoke it from AbstractService. Any implementation of Service that did not extend that base class would still have the ability to invoke the event list.

          Other enhancements

          1. clone the list before making the notifications to make the iterator robust against concurrency problems.
          2. add a package-scoped resetGlobalListeners() method for testing, to ensure the list is empty after each test run.

          Such a patch will have to wait for MAPREDUCE-3970, so I'll mark this as a dependency.

          Show
          Steve Loughran added a comment - Patch with test. I'm submitting to see what Jenkins says, but I've realised that adding the listeners to AbstractService isn't the right place to add a list of static listeners. It would be better to move the code to the ServiceOperations class of MAPREDUCE-3970 , and then invoke it from AbstractService . Any implementation of Service that did not extend that base class would still have the ability to invoke the event list. Other enhancements clone the list before making the notifications to make the iterator robust against concurrency problems. add a package-scoped resetGlobalListeners() method for testing, to ensure the list is empty after each test run. Such a patch will have to wait for MAPREDUCE-3970 , so I'll mark this as a dependency.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12517994/MAPREDUCE-3995.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 4 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed unit tests in .

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2035//testReport/
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2035//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12517994/MAPREDUCE-3995.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2035//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2035//console This message is automatically generated.
          Hide
          Steve Loughran added a comment -

          will rework and resub once MR-3970 is committed

          Show
          Steve Loughran added a comment - will rework and resub once MR-3970 is committed
          Hide
          Steve Loughran added a comment -

          This version of the patch places the global listeners into the ServiceOperations class and makes it robust against changes in the list during the notification process.

          Show
          Steve Loughran added a comment - This version of the patch places the global listeners into the ServiceOperations class and makes it robust against changes in the list during the notification process.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12518453/MAPREDUCE-3995.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 4 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 javac. The applied patch generated 506 javac compiler warnings (more than the trunk's current 505 warnings).

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.mapred.TestWritableJobConf

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2056//testReport/
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2056//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12518453/MAPREDUCE-3995.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 506 javac compiler warnings (more than the trunk's current 505 warnings). +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.mapred.TestWritableJobConf +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2056//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2056//console This message is automatically generated.
          Hide
          Steve Loughran added a comment -

          suppress warning of unchecked cast of cloned list

          Show
          Steve Loughran added a comment - suppress warning of unchecked cast of cloned list
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12518462/MAPREDUCE-3995.patch
          against trunk revision .

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 4 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.mapred.TestWritableJobConf

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2058//testReport/
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2058//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12518462/MAPREDUCE-3995.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.mapred.TestWritableJobConf +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2058//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2058//console This message is automatically generated.
          Hide
          Steve Loughran added a comment -

          TestWritableJobConf failure appears independent

          Show
          Steve Loughran added a comment - TestWritableJobConf failure appears independent
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12518462/MAPREDUCE-3995.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/184//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12518462/MAPREDUCE-3995.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-YARN-Build/184//console This message is automatically generated.
          Hide
          Steve Loughran added a comment -

          resolving as duplicate of YARN-530, which contains this code & tests

          Show
          Steve Loughran added a comment - resolving as duplicate of YARN-530 , which contains this code & tests

            People

            • Assignee:
              Steve Loughran
              Reporter:
              Steve Loughran
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development