|
To avoid confusion, default config files should come from the jar file, not from a conf directory. So we should never commit these new default files to conf/, but rather start with them in src/, copied to build/classes and from there into the jar. Thus this issue will also fix HADOOP-4617, no?
For the Configuration objects created prior to loading of HDFS defaults, the properties won't be reloaded. Currently, Configuration objects are created and same being passed around. These objects if used to load hdfs specific values, they won't be able to. no? I am thinking that instead of keeping defaults in static member, will it make sense if we keep it in specific instances. This would separate things clearly; though may require more changes.
JobConf will do addResouce(mapred-default.xml) in its constructor and similarly we can create HDFSConf (extends Configuration) which will do addResource(hdfs-default.xml) The hdfs code would instantiate HDFSConf instead of Configuration. If Configuration object is passed to it for example to DistributedFileSystem, then it would wrap it in HDFSConf before looking for hdfs specific values. > For the Configuration objects created prior to loading of HDFS defaults, the properties won't be reloaded.
Hmm. I guess you're right. We could keep a static WeakHashMap listing all existing Configuration instances, and cause them to be reloaded when the list of defaults changes? > we can create HDFSConf (extends Configuration) [ ... ] That's a pattern we'd like to get away from. The problem is that applications need to be able configure HDFS, MapReduce, Pig, etc., all with a single Configuration instance. Look, for example at the next-generation MapReduce API in: http://svn.apache.org/repos/asf/hadoop/core/trunk/src/mapred/org/apache/hadoop/mapreduce/Job.java this patch:
Replaces the hadoop-default|site.xml with core-default|site.xml, mapred-default|site.xml and hdfs-default|site.xml Keeps the registry of configuration objects in Configuration. Whenever ad new default resource is added, all configuration objects are reloaded TODO: If we want to get this into 0.20, let's file a separate issue for the documentation, as documentation changes can be made after the feature freeze tomorrow. It would be good to get this into 0.20, no?
Apart from documentation, I think there are few things which needs to ironed out before this gets into:
upated to trunk.
moved *-default.xml files to respective src folder fixed default resource loading as mentioned in above comment made minor changes in failmon and chukwa Back-compatibility is a concern. I think we should still always look for and load hadoop-site.xml but not hadoop-default.xml. When we find and load hadoop-site.xml we should emit a warning.
Attaching the patch for review.
updated to trunk and fixed a javadoc warning.
To give some time (read - past this release) for clients like HOD which may be reasonably impacted by this change, can we avoid this warning from becoming too verbose ? That is, instead of printing it everytime a configuration object is loaded, can we emit it only once ? Some minor nits:
1) Move the property dfs.block.replication from src/c+/libhdfs/tests/conf/core-site.xml to src/c+/libhdfs/tests/conf/hdfs-site.xml 2) Have a static loading for the conf resources in JobTracker and TaskTracker as well Incorporated feedback by Hemanth and Devaraj.
All tests and test-patch passed. I just committed this. Thanks, Sharad! (Please add a Release Note)
The forrest doc should be updated.
These tests seem failing after the patch:
[junit] Running org.apache.hadoop.fs.TestCopyFiles
[junit] Tests run: 12, Failures: 2, Errors: 8, Time elapsed: 164.049 sec
[junit] Running org.apache.hadoop.fs.TestDFSIO
[junit] Tests run: 1, Failures: 0, Errors: 1, Time elapsed: 10.468 sec
...
[junit] Running org.apache.hadoop.fs.TestFileSystem
[junit] Tests run: 5, Failures: 0, Errors: 2, Time elapsed: 20.726 sec
[junit] Running org.apache.hadoop.fs.TestGetFileBlockLocations
[junit] Tests run: 3, Failures: 0, Errors: 3, Time elapsed: 30.824 sec
...
[junit] Running org.apache.hadoop.hdfs.TestDFSShell
[junit] Tests run: 14, Failures: 1, Errors: 0, Time elapsed: 93.783 sec
I reverted this. The test cases fail with the patch applied, and pass when it's reverted
The svn and git ignore lists should be updated as part of this patch, too.
The tests are passing on my linux machine with the patch. Also verified with pre-reverted Revision: 726075
The problem could be user hadoop-site.xml overriding the test properties. With this patch, src/test/hadoop-site.xml is removed and respective properties are moved to core-site.xml, hdfs-site.xml and mapred-site.xml. To overcome this problem, we can keep empty src/test/hadoop-site.xml so that it overrides the conf/hadoop-site.xml and user properties are not picked up while executing test cases. changes from previous patch:
all tests and test-patch passed.
I can no longer reproduce the test failures, not even after merging the original changes back in. Sharad's theory sounds like the most likely explanation.
Nicholas, can you reproduce the failures you were seeing earlier? [exec] +1 overall.
[exec] +1 @author. The patch does not contain any @author tags.
[exec] +1 tests included. The patch appears to include 51 new or modified tests.
[exec] +1 javadoc. The javadoc tool did not generate any warning messages.
[exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
[exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
[exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
Running unit tests locally. Only the following tests failed. I think they are not related to this patch.
+1 for committing this.
I just (re)committed this. Thanks Sharad
Edit release note for publication.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Here's a proposal:
. add a static 'defaultResources' field to Configuration, whose default value is ["core-default.xml"];
. add a new static method, Configuration#addDefaultResource();
. in JobConf, add 'static { Configuration.addDefaultResource("mapred-default,xml"); }'
. in DistributedFileSystem.java, NameNode.java and DataNode.java, add 'static { Configuration.addDefaultResource("hdfs-default,xml"); }' We might factor this to a common base class or somesuch.
When you first call FileSystem.get("hdfs:///", conf), your configuration won't yet have hdfs-specific default values, but, in the course of this call, they will be loaded, before any hdfs code references them. I think this is fine, since application code should not be directly referencing hdfs-specific values. If it needs to set them, it should do so through accessor methods, and these accessor methods should dereference an HDFS class and force the loading of the HDFS defaults. Does that make sense?