Uploaded image for project: 'Hadoop Map/Reduce'
  1. Hadoop Map/Reduce
  2. MAPREDUCE-6761

Regression when handling providers - invalid configuration ServiceConfiguration causes Cluster initialization failure

    Details

    • Hadoop Flags:
      Reviewed

      Description

      When a rogue org.apache.hadoop.mapreduce.protocol.ClientProtocolProvider defines a provider that is not on classpath, then the initialization is failed with the following exception:

      java.util.ServiceConfigurationError: org.apache.hadoop.mapreduce.protocol.ClientProtocolProvider: Provider org.apache.hadoop.mapred.YarnClientProtocolProvider not found
      at java.util.ServiceLoader.fail(ServiceLoader.java:239)
      at java.util.ServiceLoader.access$300(ServiceLoader.java:185)
      at java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:372)
      at java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
      at java.util.ServiceLoader$1.next(ServiceLoader.java:480)
      at org.apache.hadoop.mapreduce.Cluster.initProviderList(Cluster.java:84)
      at org.apache.hadoop.mapreduce.Cluster.initialize(Cluster.java:114)
      at org.apache.hadoop.mapreduce.Cluster.<init>(Cluster.java:108)
      at org.apache.hadoop.mapreduce.Cluster.<init>(Cluster.java:101)
      at org.apache.hadoop.mapred.JobClient.init(JobClient.java:477)
      at org.apache.hadoop.mapred.JobClient.<init>(JobClient.java:455)

      This regression is caused by MAPREDUCE-6473

      1. MAPREDUCE-6761.patch
        2 kB
        Peter Vary
      2. MAPREDUCE-6761.2.patch
        6 kB
        Peter Vary
      3. MAPREDUCE-6761.3.patch
        13 kB
        Peter Vary
      4. MAPREDUCE-6761.4.patch
        6 kB
        Peter Vary
      5. MAPREDUCE-6761.5.patch
        6 kB
        Peter Vary

        Issue Links

          Activity

          Hide
          pvary Peter Vary added a comment -

          Please Ray Chiang, Daniel Templeton, Haibo Chen could you take a look at this patch?

          Thanks,
          Peter

          Show
          pvary Peter Vary added a comment - Please Ray Chiang , Daniel Templeton , Haibo Chen could you take a look at this patch? Thanks, Peter
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s 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 mvninstall 7m 25s trunk passed
          +1 compile 0m 24s trunk passed
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 30s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 51s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 23s the patch passed
          +1 compile 0m 21s the patch passed
          +1 javac 0m 21s the patch passed
          +1 checkstyle 0m 14s the patch passed
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 57s the patch passed
          +1 javadoc 0m 19s the patch passed
          +1 unit 2m 4s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          15m 57s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824483/MAPREDUCE-6761.patch
          JIRA Issue MAPREDUCE-6761
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7eec07ebdaed 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c5c3e81
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6679/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6679/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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 mvninstall 7m 25s trunk passed +1 compile 0m 24s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 30s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 51s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 23s the patch passed +1 compile 0m 21s the patch passed +1 javac 0m 21s the patch passed +1 checkstyle 0m 14s the patch passed +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 57s the patch passed +1 javadoc 0m 19s the patch passed +1 unit 2m 4s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 15m 57s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824483/MAPREDUCE-6761.patch JIRA Issue MAPREDUCE-6761 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7eec07ebdaed 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c5c3e81 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6679/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6679/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          pvary Peter Vary added a comment -

          Tested manually with a prepared service provider file.

          The patch is about adding a quick try {} catch {} only.

          Show
          pvary Peter Vary added a comment - Tested manually with a prepared service provider file. The patch is about adding a quick try {} catch {} only.
          Hide
          rchiang Ray Chiang added a comment -

          Thanks Peter Vary for your contribution. My comments:

          1. Please do not use the Java 1.2 Iterator interface in a while loop. You should keep the JDK5 foreach loop and put the try catch around the loop.
          2. Please change the initProviderList() method to public and mark it as @VisibleForTesting. Then, you can add a unit test for this case.
          Show
          rchiang Ray Chiang added a comment - Thanks Peter Vary for your contribution. My comments: Please do not use the Java 1.2 Iterator interface in a while loop. You should keep the JDK5 foreach loop and put the try catch around the loop. Please change the initProviderList() method to public and mark it as @VisibleForTesting. Then, you can add a unit test for this case.
          Hide
          rchiang Ray Chiang added a comment -

          One more nit. I'd recommend adding a more detailed error message letting the user know how to fix the problem as well.

          Show
          rchiang Ray Chiang added a comment - One more nit. I'd recommend adding a more detailed error message letting the user know how to fix the problem as well.
          Hide
          pvary Peter Vary added a comment -

          The iteratior presumably allows us to continue with the loop if one check is failed with an exception. I do not see the possibility to this with the foreach. Any pointers?

          Show
          pvary Peter Vary added a comment - The iteratior presumably allows us to continue with the loop if one check is failed with an exception. I do not see the possibility to this with the foreach. Any pointers?
          Hide
          templedf Daniel Templeton added a comment -

          Thanks for the patch, Peter Vary!

          Ray Chiang, using the JDK5 foreach is exactly the error Kuhu Shukla made that created the issue. The Iterator that ServiceLoader.iterator() returns can throw a ServiceConfigurationError on the next() and hasNext() calls. Peter Vary, looks like you should also guard the hasNext() call.

          I agree that the error message should be more prescriptive. I also agree that adding a test would be a good thing. You don't have to make it public, though. Default visibility (with @VisibleForTesting) should be enough.

          Show
          templedf Daniel Templeton added a comment - Thanks for the patch, Peter Vary ! Ray Chiang , using the JDK5 foreach is exactly the error Kuhu Shukla made that created the issue. The Iterator that ServiceLoader.iterator() returns can throw a ServiceConfigurationError on the next() and hasNext() calls. Peter Vary , looks like you should also guard the hasNext() call. I agree that the error message should be more prescriptive. I also agree that adding a test would be a good thing. You don't have to make it public, though. Default visibility (with @VisibleForTesting ) should be enough.
          Hide
          pvary Peter Vary added a comment -

          Thanks Ray Chiang, Daniel Templeton for the reviews.
          Added a TestCluster testcase, to check the ServiceLoader handling

          About the for/iterator issue:
          First I was leaning more to the iterator version, since when I checked the source code of the ServiceLoader, the hasNext() only loaded the configuration files, and the next() instantiated the actual objects (The exception is thrown only in the next() method). So if there was a line with an error in the configuration file, it was still possible to get the next element, and continue with the parsing. If the following lines were correct, those could be used too.

          After Daniel Templeton comment, I checked the documentation too, and found this:

          Its hasNext and next methods can therefore throw a ServiceConfigurationError
          if a provider-configuration file violates the specified format
          [..] 
          If such an error is thrown then subsequent invocations of the iterator will make
          a best effort to locate and instantiate the next available provider, but in general
          such recovery cannot be guaranteed.
          

          Which basically means, it is possible that the getNext could throw an exception, and the iterator might be stuck - this could cause infinite loop when trying to iterate through the services.

          So based on these information, I decided to stay on the safe side, and do not use features found in the implementation, but only stated as “best effort” in the documentation, since these might be changed without any notification. This means, that the parsing of the configuration files will be done until the first erroneous element found - as it was before MAPREDUCE-6473. And I can use the foreach loop with a try/catch to handle the exception.
          The problem with the MAPREDUCE-6473 was that when the initProviderList throws an exception it was not catched at all, so the Providers loaded before the error ware not used, and the Cluster creation is failed.

          Thanks again,
          Peter

          PS: I love and hate your nitpicking infrastructure "You should put an '.' at the end of your sentence in the comment"

          Show
          pvary Peter Vary added a comment - Thanks Ray Chiang , Daniel Templeton for the reviews. Added a TestCluster testcase, to check the ServiceLoader handling About the for/iterator issue: First I was leaning more to the iterator version, since when I checked the source code of the ServiceLoader, the hasNext() only loaded the configuration files, and the next() instantiated the actual objects (The exception is thrown only in the next() method). So if there was a line with an error in the configuration file, it was still possible to get the next element, and continue with the parsing. If the following lines were correct, those could be used too. After Daniel Templeton comment, I checked the documentation too, and found this: Its hasNext and next methods can therefore throw a ServiceConfigurationError if a provider-configuration file violates the specified format [..] If such an error is thrown then subsequent invocations of the iterator will make a best effort to locate and instantiate the next available provider, but in general such recovery cannot be guaranteed. Which basically means, it is possible that the getNext could throw an exception, and the iterator might be stuck - this could cause infinite loop when trying to iterate through the services. So based on these information, I decided to stay on the safe side, and do not use features found in the implementation, but only stated as “best effort” in the documentation, since these might be changed without any notification. This means, that the parsing of the configuration files will be done until the first erroneous element found - as it was before MAPREDUCE-6473 . And I can use the foreach loop with a try/catch to handle the exception. The problem with the MAPREDUCE-6473 was that when the initProviderList throws an exception it was not catched at all, so the Providers loaded before the error ware not used, and the Cluster creation is failed. Thanks again, Peter PS: I love and hate your nitpicking infrastructure "You should put an '.' at the end of your sentence in the comment"
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 6m 50s trunk passed
          +1 compile 0m 23s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 28s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 47s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 22s the patch passed
          +1 compile 0m 20s the patch passed
          +1 javac 0m 20s the patch passed
          +1 checkstyle 0m 13s the patch passed
          +1 mvnsite 0m 26s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 0m 52s the patch passed
          +1 javadoc 0m 19s the patch passed
          +1 unit 2m 2s hadoop-mapreduce-client-core in the patch passed.
          0 asflicense 0m 6s ASF License check generated no output?
          14m 55s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824507/MAPREDUCE-6761.2.patch
          JIRA Issue MAPREDUCE-6761
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
          uname Linux 04b047ed885d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 8179f9a
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6681/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6681/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 6m 50s trunk passed +1 compile 0m 23s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 28s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 47s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 20s the patch passed +1 javac 0m 20s the patch passed +1 checkstyle 0m 13s the patch passed +1 mvnsite 0m 26s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 0m 52s the patch passed +1 javadoc 0m 19s the patch passed +1 unit 2m 2s hadoop-mapreduce-client-core in the patch passed. 0 asflicense 0m 6s ASF License check generated no output? 14m 55s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824507/MAPREDUCE-6761.2.patch JIRA Issue MAPREDUCE-6761 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux 04b047ed885d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 8179f9a Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6681/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6681/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -
          1. In Cluster.java, please change back the import from wildcard to non-wildcard. This is required by the Coding Style Guide.
          2. In the unit test, I think it should be possible to modify the configuration and/or mock the class instead of creating resources.
          Show
          rchiang Ray Chiang added a comment - In Cluster.java, please change back the import from wildcard to non-wildcard. This is required by the Coding Style Guide . In the unit test, I think it should be possible to modify the configuration and/or mock the class instead of creating resources.
          Hide
          rchiang Ray Chiang added a comment -

          Kuhu Shukla and/or Jason Lowe, let me know if you see any issues with this follow up. Thanks!

          Show
          rchiang Ray Chiang added a comment - Kuhu Shukla and/or Jason Lowe , let me know if you see any issues with this follow up. Thanks!
          Hide
          kshukla Kuhu Shukla added a comment -

          Thank you Peter Vary for catching this issue and the patch. The try-catch change looks good to me. Agree with Ray Chiang on test changes.

          Show
          kshukla Kuhu Shukla added a comment - Thank you Peter Vary for catching this issue and the patch. The try-catch change looks good to me. Agree with Ray Chiang on test changes.
          Hide
          jlowe Jason Lowe added a comment -

          Thanks for the report and patch, Peter Vary! Apologies this was missed in MAPREDUCE-6473.

          One potential issue is if a user explicitly placed a service provider ahead of the built-in service providers with the intent of overriding them. If that provider doesn't load then we will log a message but proceed anyway. At that point we should have failed, as would have even pre-MAPREDUCE-6473, but we end up proceeding without using the user's expected protocol provider. The reason we didn't see this kind of error before MAPREDUCE-6473 is because the error occurs after a "good" protocol provider, so we never tried to load it. After MAPREDUCE-6473 all the protocol providers are loaded up front, so we found the latent error.

          So even after this proposed fix there's a potentially unexpected change in behavior. To preserve the original behavior, initProviderList should stop iterating the service providers when one of them throws. Then we can proceed to the Cluster code that iterates the providers array we were able to load. If one of them fits then we're good, otherwise we throw.

          Other comments on the patch:
          The core dependency on common creates a cycle because common already depends upon core:

          [ERROR] The projects in the reactor contain a cyclic reference: Edge between 'Vertex{label='org.apache.hadoop:hadoop-mapreduce-client-common:3.0.0-alpha2-SNAPSHOT'}' and 'Vertex{label='org.apache.hadoop:hadoop-mapreduce-client-core:3.0.0-alpha2-SNAPSHOT'}' introduces to cycle in the graph org.apache.hadoop:hadoop-mapreduce-client-core:3.0.0-alpha2-SNAPSHOT --> org.apache.hadoop:hadoop-mapreduce-client-common:3.0.0-alpha2-SNAPSHOT --> org.apache.hadoop:hadoop-mapreduce-client-core:3.0.0-alpha2-SNAPSHOT -> [Help 1]
          

          Not sure why the precommit build didn't catch this. Should be trivial to create a stubbed protocol provider class in the test that always fits and use that instead to remove the dependency.

          Nit: It's unnecessary to add the client protocol provider as a field in the Cluster object just for testing. If we use a custom protocol provider for test as suggested above, that protocol provider can return a custom client protocol class that we can test for via the existing Cluster#getClient method to know whether it used the expected protocol provider. Not a must-fix.

          Show
          jlowe Jason Lowe added a comment - Thanks for the report and patch, Peter Vary ! Apologies this was missed in MAPREDUCE-6473 . One potential issue is if a user explicitly placed a service provider ahead of the built-in service providers with the intent of overriding them. If that provider doesn't load then we will log a message but proceed anyway. At that point we should have failed, as would have even pre- MAPREDUCE-6473 , but we end up proceeding without using the user's expected protocol provider. The reason we didn't see this kind of error before MAPREDUCE-6473 is because the error occurs after a "good" protocol provider, so we never tried to load it. After MAPREDUCE-6473 all the protocol providers are loaded up front, so we found the latent error. So even after this proposed fix there's a potentially unexpected change in behavior. To preserve the original behavior, initProviderList should stop iterating the service providers when one of them throws. Then we can proceed to the Cluster code that iterates the providers array we were able to load. If one of them fits then we're good, otherwise we throw. Other comments on the patch: The core dependency on common creates a cycle because common already depends upon core: [ERROR] The projects in the reactor contain a cyclic reference: Edge between 'Vertex{label='org.apache.hadoop:hadoop-mapreduce-client-common:3.0.0-alpha2-SNAPSHOT'}' and 'Vertex{label='org.apache.hadoop:hadoop-mapreduce-client-core:3.0.0-alpha2-SNAPSHOT'}' introduces to cycle in the graph org.apache.hadoop:hadoop-mapreduce-client-core:3.0.0-alpha2-SNAPSHOT --> org.apache.hadoop:hadoop-mapreduce-client-common:3.0.0-alpha2-SNAPSHOT --> org.apache.hadoop:hadoop-mapreduce-client-core:3.0.0-alpha2-SNAPSHOT -> [Help 1] Not sure why the precommit build didn't catch this. Should be trivial to create a stubbed protocol provider class in the test that always fits and use that instead to remove the dependency. Nit: It's unnecessary to add the client protocol provider as a field in the Cluster object just for testing. If we use a custom protocol provider for test as suggested above, that protocol provider can return a custom client protocol class that we can test for via the existing Cluster#getClient method to know whether it used the expected protocol provider. Not a must-fix.
          Hide
          pvary Peter Vary added a comment -

          Thank you for the comments!

          Here is what I have done in this patch:
          1. Checked the imports, and removed wildcards as Ray Chiang commented.
          2. Created Dummy providers, and protocols, and removed the dependency on client-common as Jason Lowe commented
          3. This version of the patch uses the foreach to iterate through the elements, and the unit test checks that the next() is not called after the first error, so I think the result will be the same as before the previous patch. Jason Lowe, could you please verify this
          4. I had to change the type of the frameworkLoader to its' interface (Iterable), so I can mock it with mockito. We do not use any ServiceLoader specific methods in the Cluster class, and it is compiling and running now. Still I do not think it is a good solution. If anyone has a better idea, please share, or if you find this acceptable tradeoff then share that too.

          Thanks for the helpful reviews all!

          Peter

          Show
          pvary Peter Vary added a comment - Thank you for the comments! Here is what I have done in this patch: 1. Checked the imports, and removed wildcards as Ray Chiang commented. 2. Created Dummy providers, and protocols, and removed the dependency on client-common as Jason Lowe commented 3. This version of the patch uses the foreach to iterate through the elements, and the unit test checks that the next() is not called after the first error, so I think the result will be the same as before the previous patch. Jason Lowe , could you please verify this 4. I had to change the type of the frameworkLoader to its' interface (Iterable), so I can mock it with mockito. We do not use any ServiceLoader specific methods in the Cluster class, and it is compiling and running now. Still I do not think it is a good solution. If anyone has a better idea, please share, or if you find this acceptable tradeoff then share that too. Thanks for the helpful reviews all! Peter
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 8m 0s trunk passed
          +1 compile 0m 26s trunk passed
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 33s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 1s trunk passed
          +1 javadoc 0m 24s trunk passed
          +1 mvninstall 0m 28s the patch passed
          +1 compile 0m 26s the patch passed
          +1 javac 0m 26s the patch passed
          +1 checkstyle 0m 15s the patch passed
          +1 mvnsite 0m 30s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 1m 4s the patch passed
          +1 javadoc 0m 22s the patch passed
          +1 unit 2m 18s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          17m 48s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824622/MAPREDUCE-6761.3.patch
          JIRA Issue MAPREDUCE-6761
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
          uname Linux e15557578b66 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 03a9343
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6683/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6683/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 8m 0s trunk passed +1 compile 0m 26s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 33s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 1s trunk passed +1 javadoc 0m 24s trunk passed +1 mvninstall 0m 28s the patch passed +1 compile 0m 26s the patch passed +1 javac 0m 26s the patch passed +1 checkstyle 0m 15s the patch passed +1 mvnsite 0m 30s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 1m 4s the patch passed +1 javadoc 0m 22s the patch passed +1 unit 2m 18s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 17m 48s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824622/MAPREDUCE-6761.3.patch JIRA Issue MAPREDUCE-6761 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux e15557578b66 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 03a9343 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6683/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6683/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          kshukla Kuhu Shukla added a comment -

          I had to change the type of the frameworkLoader to its' interface (Iterable), so I can mock it with mockito.

          Not sure on how to handle this, but here are some changes we can make to the test.
          Remove DummyClientProtocolProvider and use a ClientProtocolProvider with mock ClientProtocol in TestCluster.

            public ClientProtocolProvider getClientProtocolProvider() {
              return new ClientProtocolProvider() {
                @Override
                public ClientProtocol create(Configuration conf) throws IOException {
                  return mock(ClientProtocol.class);
                }
          
                @Override
                public ClientProtocol create(InetSocketAddress addr, Configuration
                    conf) throws IOException {
                  return mock(ClientProtocol.class);
                }
          
                @Override
                public void close(ClientProtocol clientProtocol) throws IOException {
          
                }
              };
            }
          

          and use this in the return/throw mock chain

           ClientProtocolProvider clientProtocolProvider1 = getClientProtocolProvider();
              ClientProtocolProvider clientProtocolProvider2 = getClientProtocolProvider();
              when(iterator.next()).thenReturn(clientProtocolProvider1)
                  .thenThrow(new ServiceConfigurationError("Test error"))
                  .thenReturn(clientProtocolProvider2);
          ......
          assertThat("Unexpected type of clientProtocolProvider",
                  testCluster.getClient(),
                  instanceOf(ClientProtocol.class));
              // Check if we do not try to load the providers after a failure.
              verify(iterator, times(2)).next();
          

          Also, I think we don't need the pom.xml change. I seem to be able to run this without it.
          Hope this helps Peter Vary.

          Show
          kshukla Kuhu Shukla added a comment - I had to change the type of the frameworkLoader to its' interface (Iterable), so I can mock it with mockito. Not sure on how to handle this, but here are some changes we can make to the test. Remove DummyClientProtocolProvider and use a ClientProtocolProvider with mock ClientProtocol in TestCluster. public ClientProtocolProvider getClientProtocolProvider() { return new ClientProtocolProvider() { @Override public ClientProtocol create(Configuration conf) throws IOException { return mock(ClientProtocol.class); } @Override public ClientProtocol create(InetSocketAddress addr, Configuration conf) throws IOException { return mock(ClientProtocol.class); } @Override public void close(ClientProtocol clientProtocol) throws IOException { } }; } and use this in the return/throw mock chain ClientProtocolProvider clientProtocolProvider1 = getClientProtocolProvider(); ClientProtocolProvider clientProtocolProvider2 = getClientProtocolProvider(); when(iterator.next()).thenReturn(clientProtocolProvider1) .thenThrow( new ServiceConfigurationError( "Test error" )) .thenReturn(clientProtocolProvider2); ...... assertThat( "Unexpected type of clientProtocolProvider" , testCluster.getClient(), instanceOf(ClientProtocol.class)); // Check if we do not try to load the providers after a failure. verify(iterator, times(2)).next(); Also, I think we don't need the pom.xml change. I seem to be able to run this without it. Hope this helps Peter Vary .
          Hide
          pvary Peter Vary added a comment -

          Thanks Kuhu Shukla for the suggestion!

          Using your stuff, I updated the patch with the following 2 changes above the previous one:
          1, Added Kuhu Shukla's suggestion for the test classes - the test is much nicer now.
          2, Removed the explicit mockito dependency - although I am not sure about this, since we are using it here. I think it is project dependent how we handle these kind of dependencies - call out if you think it is better to add it back.

          Peter

          Show
          pvary Peter Vary added a comment - Thanks Kuhu Shukla for the suggestion! Using your stuff, I updated the patch with the following 2 changes above the previous one: 1, Added Kuhu Shukla 's suggestion for the test classes - the test is much nicer now. 2, Removed the explicit mockito dependency - although I am not sure about this, since we are using it here. I think it is project dependent how we handle these kind of dependencies - call out if you think it is better to add it back. Peter
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 43s trunk passed
          +1 compile 0m 24s trunk passed
          +1 checkstyle 0m 18s trunk passed
          +1 mvnsite 0m 30s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 0m 51s trunk passed
          +1 javadoc 0m 23s trunk passed
          +1 mvninstall 0m 25s the patch passed
          +1 compile 0m 24s the patch passed
          +1 javac 0m 24s the patch passed
          +1 checkstyle 0m 14s the patch passed
          +1 mvnsite 0m 28s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 59s the patch passed
          +1 javadoc 0m 21s the patch passed
          +1 unit 2m 17s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 15s The patch does not generate ASF License warnings.
          16m 56s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824638/MAPREDUCE-6761.4.patch
          JIRA Issue MAPREDUCE-6761
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 304234c16fed 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 763f049
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6684/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6684/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 43s trunk passed +1 compile 0m 24s trunk passed +1 checkstyle 0m 18s trunk passed +1 mvnsite 0m 30s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 0m 51s trunk passed +1 javadoc 0m 23s trunk passed +1 mvninstall 0m 25s the patch passed +1 compile 0m 24s the patch passed +1 javac 0m 24s the patch passed +1 checkstyle 0m 14s the patch passed +1 mvnsite 0m 28s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 59s the patch passed +1 javadoc 0m 21s the patch passed +1 unit 2m 17s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 16m 56s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824638/MAPREDUCE-6761.4.patch JIRA Issue MAPREDUCE-6761 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 304234c16fed 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 763f049 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6684/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6684/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rchiang Ray Chiang added a comment -

          Thanks for the cleanup Kuhu Shukla and Peter Vary. The current patch looks good to me. +1

          Does anyone else have any follow up comments?

          Show
          rchiang Ray Chiang added a comment - Thanks for the cleanup Kuhu Shukla and Peter Vary . The current patch looks good to me. +1 Does anyone else have any follow up comments?
          Hide
          jlowe Jason Lowe added a comment -

          Thanks for updating the patch. Looks good overall with a minor nit in the test:

              // Check that we get the acceptable client, even after
              // failure in instantiation.
              assertThat("Unexpected type of clientProtocolProvider",
                  testCluster.getClient(),
                  instanceOf(ClientProtocol.class));
          

          The ServiceLoader is only going to load classes that can be cast to a ClientProtocol, so if the Cluster constructor doesn't fail then this too will never fail. In addition ClientProtocol is the return type for the Cluster#getClient method, so this is essentially a complicated null check since that's the only return value that could cause the test to fail. It also isn't testing which client protocol type we received, since the mock always returns the same type when it isn't throwing an error. The test would be more robust if the mock could return different derived classes from ClientProtocol and verify we received the correct one.

          Fairly minor nit though, so I'm still +1 on the current patch if that isn't addressed.

          Show
          jlowe Jason Lowe added a comment - Thanks for updating the patch. Looks good overall with a minor nit in the test: // Check that we get the acceptable client, even after // failure in instantiation. assertThat( "Unexpected type of clientProtocolProvider" , testCluster.getClient(), instanceOf(ClientProtocol.class)); The ServiceLoader is only going to load classes that can be cast to a ClientProtocol, so if the Cluster constructor doesn't fail then this too will never fail. In addition ClientProtocol is the return type for the Cluster#getClient method, so this is essentially a complicated null check since that's the only return value that could cause the test to fail. It also isn't testing which client protocol type we received, since the mock always returns the same type when it isn't throwing an error. The test would be more robust if the mock could return different derived classes from ClientProtocol and verify we received the correct one. Fairly minor nit though, so I'm still +1 on the current patch if that isn't addressed.
          Hide
          pvary Peter Vary added a comment -

          Thanks Jason Lowe for the comment.

          I decided not to check the selected protocolprovider class explicitly, since the goal of the test is to validate the exception handling, and not to validate the protocolprovider selection logic itself. To validate the selected provider class itself (when we have multiple possible providers) would be a different testcase altogether.

          With that in mind, I changed the type check to a null check, so it would be easier to read the code, and it expresses better the decision I have made above.

          Thanks,
          Peter

          Show
          pvary Peter Vary added a comment - Thanks Jason Lowe for the comment. I decided not to check the selected protocolprovider class explicitly, since the goal of the test is to validate the exception handling, and not to validate the protocolprovider selection logic itself. To validate the selected provider class itself (when we have multiple possible providers) would be a different testcase altogether. With that in mind, I changed the type check to a null check, so it would be easier to read the code, and it expresses better the decision I have made above. Thanks, Peter
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 38s trunk passed
          +1 compile 0m 23s trunk passed
          +1 checkstyle 0m 16s trunk passed
          +1 mvnsite 0m 28s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 45s trunk passed
          +1 javadoc 0m 21s trunk passed
          +1 mvninstall 0m 22s the patch passed
          +1 compile 0m 22s the patch passed
          +1 javac 0m 22s the patch passed
          +1 checkstyle 0m 13s the patch passed
          +1 mvnsite 0m 25s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 52s the patch passed
          +1 javadoc 0m 18s the patch passed
          +1 unit 2m 5s hadoop-mapreduce-client-core in the patch passed.
          +1 asflicense 0m 14s The patch does not generate ASF License warnings.
          15m 8s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824927/MAPREDUCE-6761.5.patch
          JIRA Issue MAPREDUCE-6761
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 98da27d4c254 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 22fc46d
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6686/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6686/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 38s trunk passed +1 compile 0m 23s trunk passed +1 checkstyle 0m 16s trunk passed +1 mvnsite 0m 28s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 45s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 22s the patch passed +1 compile 0m 22s the patch passed +1 javac 0m 22s the patch passed +1 checkstyle 0m 13s the patch passed +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 52s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 2m 5s hadoop-mapreduce-client-core in the patch passed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 15m 8s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12824927/MAPREDUCE-6761.5.patch JIRA Issue MAPREDUCE-6761 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 98da27d4c254 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 22fc46d Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6686/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6686/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          jlowe Jason Lowe added a comment -

          +1 for the latest patch. I'll commit this later today if there are no objections.

          Show
          jlowe Jason Lowe added a comment - +1 for the latest patch. I'll commit this later today if there are no objections.
          Hide
          rchiang Ray Chiang added a comment -

          +1. Updated test still passes with the fix and fails without the fix.

          Show
          rchiang Ray Chiang added a comment - +1. Updated test still passes with the fix and fails without the fix.
          Hide
          jlowe Jason Lowe added a comment -

          Thanks to Peter Vary for the contribution and to Ray Chiang, Daniel Templeton, and Kuhu Shukla for additional review! I committed this to trunk, branch-2, and branch-2.8.

          Show
          jlowe Jason Lowe added a comment - Thanks to Peter Vary for the contribution and to Ray Chiang , Daniel Templeton , and Kuhu Shukla for additional review! I committed this to trunk, branch-2, and branch-2.8.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10337 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10337/)
          MAPREDUCE-6761. Regression when handling providers - invalid (jlowe: rev 6fa9bf4407a0b09be8ce4e52d2fde89adba34f1a)

          • (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Cluster.java
          • (add) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestCluster.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10337 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10337/ ) MAPREDUCE-6761 . Regression when handling providers - invalid (jlowe: rev 6fa9bf4407a0b09be8ce4e52d2fde89adba34f1a) (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/Cluster.java (add) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapreduce/TestCluster.java

            People

            • Assignee:
              pvary Peter Vary
              Reporter:
              pvary Peter Vary
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development