Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 2.0.0-alpha
-
Fix Version/s: 2.0.2-alpha
-
Component/s: fs
-
Labels:None
-
Hadoop Flags:Reviewed
Description
The javadocs for FileSystem#getDefaultBlockSize and FileSystem#getDefaultReplication claim that "The given path will be used to locate the actual filesystem" however they both ignore the path.
Issue Links
- is related to
-
FLUME-2027
Check for default replication fails on federated cluster in hdfs sink
-
- Resolved
-
-
HBASE-6067
HBase won't start when hbase.rootdir uses ViewFileSystem
-
- Closed
-
- relates to
-
HADOOP-8014
ViewFileSystem does not correctly implement getDefaultBlockSize, getDefaultReplication, getContentSummary
-
- Closed
-
-
HADOOP-8430
Backport new FileSystem methods introduced by HADOOP-8014 to branch-1
-
- Closed
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Should we add the new FileSystem#getDefault* methods that take a Path to Hadoop 1.x?
The problem is that the ViewFileSystem defaults throw an exception now so programs like HBase break on viewfs enabled clients because they don't use the new methods that take a path (because these don't exist in Hadoop 1.x) and the versions that don't take a path don't work, eg
2012-05-17 14:02:14,328 ERROR org.apache.hadoop.hbase.regionserver.HRegionServer: Failed init org.apache.hadoop.fs.viewfs.NotInMountpointException: getDefaultBlockSize on empty path is invalid at org.apache.hadoop.fs.viewfs.ViewFileSystem.getDefaultBlockSize(ViewFileSystem.java:488) at org.apache.hadoop.hbase.regionserver.wal.HLog.<init>(HLog.java:377)
Since there's no notion of "default file system" for a viewfs mount table I agree that throwing an exception for the non-path case is the right call.
It's probably a good idea back-port to 1.x. The changes are rather minimal and low impact. The ramifications of not having the changes in 1.x are pretty bad. Viewfs will use a replication factor of 1 and the local filesystem's block size...
Out of curiousity, why is HBase querying the default block size? Does it really need to? Ie. If only to pass to FileSystem#create calls, could it use the create methods with smaller signatures that automatically get the replication factor and block size for the new path?
Re-opening and re-targeting to 1.x for a backport of replication factor and block size fixes for ViewFileSystem.
Out of curiousity, why is HBase querying the default block size?
One thing I am aware of is that it relies on default HDFS block sizes as the default threshold for HLog rolls. This value is presently loaded (for a checker thread) before a HLog is created so there's no path at that point. HBase can be fixed to use a path.
MR2 tasks too query HDFS block size presently to determine (versus the input size of a job) if a task can be made uber. MR2 does use a Path though (job submission directory presently, although thats wrong and there's a FIXME in there to use the input file instead…).
I do not quite know what ViewFileSystem does at the moment, but for non-Path queries is there no View-Root-Path it can fall back on?
Got a nit though: The calls "get*Default*Replication", etc. have 'default' in them and sorta make no sense given a path object, given also that they're non-static methods within a FileSystem API? Shouldn't they be simply getReplication, etc.? If you agree, I'll file another JIRA.
Just FYI, I made the original patch for these apis, John George added tests and took it over the finish line. Either of us can address the issues posed in this jira.
I do not quite know what ViewFileSystem does at the moment, but for non-Path queries is there no View-Root-Path it can fall back on?
Last I knew, ViewFileSystem does not support mounting a filesystem on the root (ie. /). Therefore, it's not really possible to "guess" a default since the underlying mounts may have different defaults. Even if a filesystem can be mounted on the root, returning the defaults of that fs is wrong since a union/merge/whatever-it-is-called mount of another fs may mean the intended path's resolved fs has different values. Basically a Path really needs to be supplied so the mount can be fully resolved.
Multi-mount filesystems break the semantics of these apis designed around the assumption of a single filesystem. Unfortunately this means there's going to be a hopefully not too painful bump in the road for users depending on these apis.
Unfortunately it also appears that getDefaultReplication() and getDefaultBlockSize() weren't marked as internally limited/private, yet externally deprecated. Otherwise users may have a better idea of why their code is breaking. We may want to consider adding those annotations.
Got a nit though: The calls "get*Default*Replication", etc. have 'default' in them and sorta make no sense given a path object, given also that they're non-static methods within a FileSystem API? Shouldn't they be simply getReplication, etc.? If you agree, I'll file another JIRA.
I kind of see what you are saying, but it actually does make sense given a Path object. The question it's answering is: "what are the default values for the actual filesystem that will contain this given path?".
I confused by the concern with the methods being non-static. A fs, such as ViewFileSystem must be able to override the method to resolve the path through the mount table to get the real fs. Maybe we are talking past each other, so what methods & signatures are you envisioning?
I'm confused by the concern with the methods being non-static.
Ah its not a big concern, just a neatness thing.
I meant, if one were to call:
SomeUtilsClass.getDefaultReplication(Path), a static method, and this were to do: Path.getFileSystem().getDefaultReplication() or so, that makes more correct sense (i.e. user is expecting Path to evaluate to the right FS via this static call).
However, here we do:
fileSystem.getDefaultReplication(Path), wherein a user already has a proper fileSystem instance - so may not feel the need to supply a path as well.
But this is just a nit, probably not worth changing things for now. So long as there's docs that explain why its around for, its alright.
HBase is getting the default block size and replication level at startup to set other variables (eg min replication level and log roll size). We don't want to hard code those in HBase as then we have to define them in two places and there isn't necessarily one set of defaults that works for multiple file systems.
SomeUtilsClass.getDefaultReplication(Path), a static method, and this were to do: Path.getFileSystem().getDefaultReplication()
Ah, I understand. The issue is that getFileSystem for a viewfs path will just return the viewfs fs, not the underlying fs mounted by viewfs. That's correct behavior to make viewfs transparent to the user. Hence why viewfs has to keep the underlying fs invisible, yet allow it to be queried for replication/block size.
@Eli: I completely agree. Keeping the values in sync with the filesystem, and having a scheme of managing the settings per fs, ick. Maintenance nightmare.
One reason I'm not super thrilled with backporting the new APIs from HADOOP-8014 to branch-1 and introducing uses of them in other projects like HBase is that it means new HBase releases won't work on all but the most recent releases of 1.x. This is a rare / unfortunate case where federation violates the FileSystem API (which assumes there's a default file system associated with a FileSystem object) so users who want to support federation have to give up working transparently on older releases. Don't see a great alternative though, and at least this gives people a transition path.
Here's my proposal:
- Deprecate the FileSystem#getDefault* and getServerDefault methods that don't take a Path in trunk and branch-2 since these uses aren't valid (their programs will break if the user ever switches to using viewfs)
- Add the methods that take a Path to branch-1 for the Hadoop 1.1 release so people can transition
Reasonable?
If people agree I'll deprecate the methods in this jira and file a separate jira to backport the methods to branch-1.
Sounds reasonable to me. You'll probably want to throw up, but could you preserve backwards-compat by using reflection to see if the Path method is there?
Yea, on the user-side you could use reflection (or catch NoSuchMethodException). Yuck. HBase alreayd does this to check which sync methods are supported.
Patch attached. Deprecates getServerDefaults, getDefaultBlockSize and getDefaultReplication and updates users where appropriate (we have some internal ones like HttpFs that will hit the same issue).
Filed to HADOOP-8430 for the branch-1 change.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12528824/hadoop-8422.txt
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 6 new or modified test files.
-1 javac. The applied patch generated 1997 javac compiler warnings (more than the trunk's current 1972 warnings).
+1 javadoc. The javadoc tool did not generate any warning messages.
+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 in hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs hadoop-hdfs-project/hadoop-hdfs-httpfs:
org.apache.hadoop.fs.viewfs.TestViewFsTrash
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1026//testReport/
Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/1026//artifact/trunk/trunk/patchprocess/diffJavacWarnings.txt
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1026//console
This message is automatically generated.
+1
Thansk Daryn, Harsh and Tucu. I've committed and merged to branch-2.
I forgot to mention TestViewFsTrash is unrelated and the new warnings are due to uses of the newly deprecated method that can't be modified to use the new methods that take a path.
Integrated in Hadoop-Hdfs-trunk #1056 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1056/)
HADOOP-8422. Deprecate FileSystem#getDefault* and getServerDefault methods that don't take a Path argument. Contributed by Eli Collins (Revision 1342495)
Result = SUCCESS
eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342495
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemTestHelper.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/S3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestQuota.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
Integrated in Hadoop-Mapreduce-trunk #1090 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1090/)
HADOOP-8422. Deprecate FileSystem#getDefault* and getServerDefault methods that don't take a Path argument. Contributed by Eli Collins (Revision 1342495)
Result = ABORTED
eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342495
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemTestHelper.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/S3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestQuota.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
Integrated in Hadoop-Hdfs-trunk-Commit #2361 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2361/)
HADOOP-8422. Deprecate FileSystem#getDefault* and getServerDefault methods that don't take a Path argument. Contributed by Eli Collins (Revision 1342495)
Result = SUCCESS
eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342495
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemTestHelper.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/S3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestQuota.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
Integrated in Hadoop-Common-trunk-Commit #2288 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2288/)
HADOOP-8422. Deprecate FileSystem#getDefault* and getServerDefault methods that don't take a Path argument. Contributed by Eli Collins (Revision 1342495)
Result = SUCCESS
eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342495
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemTestHelper.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/S3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestQuota.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
Integrated in Hadoop-Mapreduce-trunk-Commit #2307 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2307/)
HADOOP-8422. Deprecate FileSystem#getDefault* and getServerDefault methods that don't take a Path argument. Contributed by Eli Collins (Revision 1342495)
Result = FAILURE
eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1342495
Files :
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemTestHelper.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/S3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/FSOperations.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestQuota.java
- /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java
The default impls are intended to ignore the path by default, but a multi-fs like ViewFileSystem overrides it to query the underlying filesystem for the given path.