Hadoop Common
  1. Hadoop Common
  2. HADOOP-6227

Configuration does not lock parameters marked final if they have no value.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: conf
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      A use-case: Recently, we stumbled on a bug that wanted us to disable the feature to run a debug script in map/reduce on tasks that fail, specified by mapred.

      {map|reduce}

      .task.debug.script. The best way of disabling it seemed to be to set it with no value in the cluster configuration and mark it final. This did not work, however, because the code in configuration explicitly checks if the value is not null before adding it to the list of final parameters.

      Without an ability to do this, there might be a need to explicitly have a special way of disabling optional features.

      1. patch-6227.txt
        3 kB
        Amareshwari Sriramadasu
      2. patch-6227-0.20.txt
        2 kB
        Amareshwari Sriramadasu
      3. patch-6227-ydist.txt
        3 kB
        Vinod Kumar Vavilapalli

        Activity

        Hemanth Yamijala created issue -
        Hide
        Hemanth Yamijala added a comment -

        The code in configuration looks like this:

                // Ignore this parameter if it has already been marked as 'final'
                if (attr != null && value != null) {
                  if (!finalParameters.contains(attr)) {
                    properties.setProperty(attr, value);
                    if (storeResource) {
                      updatingResource.put(attr, name.toString());
                    }
                    if (finalParameter)
                      finalParameters.add(attr);
                  } else {
                    LOG.warn(name+":a attempt to override final parameter: "+attr
                             +";  Ignoring.");
                  }
                }
        

        While the check for value being null is required before adding the property to the 'properties' instance, adding it to finalParameters or for that matter even updatingResource shouldn't be an issue even if the value is null. This would certainly fix the current issue. Thoughts ?

        Show
        Hemanth Yamijala added a comment - The code in configuration looks like this: // Ignore this parameter if it has already been marked as ' final ' if (attr != null && value != null ) { if (!finalParameters.contains(attr)) { properties.setProperty(attr, value); if (storeResource) { updatingResource.put(attr, name.toString()); } if (finalParameter) finalParameters.add(attr); } else { LOG.warn(name+ ":a attempt to override final parameter: " +attr + "; Ignoring." ); } } While the check for value being null is required before adding the property to the 'properties' instance, adding it to finalParameters or for that matter even updatingResource shouldn't be an issue even if the value is null. This would certainly fix the current issue. Thoughts ?
        Hide
        Amareshwari Sriramadasu added a comment -

        Patch with the fix

        Show
        Amareshwari Sriramadasu added a comment - Patch with the fix
        Amareshwari Sriramadasu made changes -
        Field Original Value New Value
        Attachment patch-6227-0.20.txt [ 12418237 ]
        Attachment patch-6227.txt [ 12418238 ]
        Amareshwari Sriramadasu made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Fix Version/s 0.20.1 [ 12313866 ]
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418238/patch-6227.txt
        against trunk revision 809491.

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

        +1 tests included. The patch appears to include 3 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 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/4/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/4/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/4/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/4/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/12418238/patch-6227.txt against trunk revision 809491. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/4/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/4/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/4/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/4/console This message is automatically generated.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        Yahoo! distribution patch. The patch for branch 0.20 will not apply to this distribution with HADOOP-6184 already in.

        Show
        Vinod Kumar Vavilapalli added a comment - Yahoo! distribution patch. The patch for branch 0.20 will not apply to this distribution with HADOOP-6184 already in.
        Vinod Kumar Vavilapalli made changes -
        Attachment patch-6227-ydist.txt [ 12418242 ]
        Hide
        Hemanth Yamijala added a comment -

        Code changes look fine to me. +1.

        Show
        Hemanth Yamijala added a comment - Code changes look fine to me. +1.
        Hide
        Vinod Kumar Vavilapalli added a comment -

        I just tested this fix on a single node cluster so as to enable/disable mapred debug script by setting it to null. The patch works as expected. A null value marked as final on server side can no longer be overridden by clients.

        Show
        Vinod Kumar Vavilapalli added a comment - I just tested this fix on a single node cluster so as to enable/disable mapred debug script by setting it to null. The patch works as expected. A null value marked as final on server side can no longer be overridden by clients.
        Hide
        Arun C Murthy added a comment -

        +1

        Show
        Arun C Murthy added a comment - +1
        Hide
        Hemanth Yamijala added a comment -

        I just committed this to trunk. Thanks, Amareshwari !

        I did not commit to the 0.20 branch, because there's an active vote in progress and did not think this existing bug should block the release. If decided otherwise, we can commit to the 0.20 branch as well.

        Show
        Hemanth Yamijala added a comment - I just committed this to trunk. Thanks, Amareshwari ! I did not commit to the 0.20 branch, because there's an active vote in progress and did not think this existing bug should block the release. If decided otherwise, we can commit to the 0.20 branch as well.
        Hemanth Yamijala made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Release Note Modified configuration to allow setting final parameters with null values.
        Assignee Amareshwari Sriramadasu [ amareshwari ]
        Fix Version/s 0.21.0 [ 12313563 ]
        Fix Version/s 0.20.1 [ 12313866 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #4 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/4/)
        . Fix Configuration to allow final parameters to be set to null and prevent them from being overridden. Contributed by Amareshwari Sriramadasu.

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #4 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/4/ ) . Fix Configuration to allow final parameters to be set to null and prevent them from being overridden. Contributed by Amareshwari Sriramadasu.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk #82 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/82/)
        . Fix Configuration to allow final parameters to be set to null and prevent them from being overridden. Contributed by Amareshwari Sriramadasu.

        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk #82 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/82/ ) . Fix Configuration to allow final parameters to be set to null and prevent them from being overridden. Contributed by Amareshwari Sriramadasu.
        Hide
        Robert Chansler added a comment -

        Editorial pass over all release notes prior to publication of 0.21. Bug.

        Show
        Robert Chansler added a comment - Editorial pass over all release notes prior to publication of 0.21. Bug.
        Robert Chansler made changes -
        Release Note Modified configuration to allow setting final parameters with null values.
        Tom White made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        1h 7m 1 Amareshwari Sriramadasu 01/Sep/09 12:38
        Patch Available Patch Available Resolved Resolved
        2h 49m 1 Hemanth Yamijala 01/Sep/09 15:27
        Resolved Resolved Closed Closed
        357d 5h 12m 1 Tom White 24/Aug/10 20:39

          People

          • Assignee:
            Amareshwari Sriramadasu
            Reporter:
            Hemanth Yamijala
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development