virajith commented on a change in pull request #1988:
URL: https://github.com/apache/hadoop/pull/1988#discussion_r418375437
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsConstants.java
##########
@@ -42,4 +42,11 @@
*/
public static final URI VIEWFS_URI = URI.create("viewfs:///");
public static final String VIEWFS_SCHEME = "viewfs";
+
+ public static final String VIEWFS_OVERLOAD_SCHEME_KEY =
+ "fs.viewfs.overload.scheme";
+ public static final String VIEWFS_OVERLOAD_SCHEME_DEFAULT = "hdfs";
+ public static final String
FS_VIEWFS_OVERLOAD_SCHEME_TARGET_FS_IMPL_PATTERN_KEY =
+ "fs.viewfs.overload.scheme.target.%s.impl";
+ public static final String FS_IMPL_PATTERN_KEY = "fs.%s.impl";
Review comment:
Why add this here? This is just used in tests right?
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFsOverloadScheme.java
##########
@@ -0,0 +1,175 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.viewfs;
+
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsConstants;
+import org.apache.hadoop.fs.UnsupportedFileSystemException;
+
+/******************************************************************************
+ * This class is extended from the ViewFileSystem for the overloaded scheme
+ * file system. This object is the way end-user code interacts with a multiple
Review comment:
nits:
"object is" -> "objective here is to handle"
"a multiple" -> multiple.
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -302,6 +320,11 @@ protected FileSystem getTargetFileSystem(final String
settings,
}
}
+ protected void superFSInit(final URI theUri, final Configuration conf)
Review comment:
javadoc for this method? This seems a bit hacky but I understand the
need. I think initializeSuperFs is a slightly better name but don't have a
strong opinion here.
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFsOverloadScheme.java
##########
@@ -0,0 +1,175 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.viewfs;
+
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsConstants;
+import org.apache.hadoop.fs.UnsupportedFileSystemException;
+
+/******************************************************************************
+ * This class is extended from the ViewFileSystem for the overloaded scheme
+ * file system. This object is the way end-user code interacts with a multiple
+ * mounted file systems transparently. Overloaded scheme based uri can be
+ * continued to use as end user interactive uri and mount links can be
+ * configured to any Hadoop compatible file system. This class maintains all
+ * the target file system instances and delegates the calls to respective
+ * target file system based on mount link mapping. Mount link configuration
+ * format and behavior is same as ViewFileSystem.
+ *****************************************************************************/
[email protected]({ "MapReduce", "HBase", "Hive" })
[email protected]
+public class ViewFsOverloadScheme extends ViewFileSystem {
+
+ public ViewFsOverloadScheme() throws IOException {
+ super();
+ }
+
+ private FsCreator fsCreator;
+ private String myScheme;
+
+ @Override
+ public String getScheme() {
+ return myScheme;
+ }
+
+ @Override
+ public void initialize(final URI theUri, final Configuration conf)
+ throws IOException {
+ superFSInit(theUri, conf);
+ setConf(conf);
+ config = conf;
+ myScheme = config.get(FsConstants.VIEWFS_OVERLOAD_SCHEME_KEY);
Review comment:
Is there a reason why the scheme of this FS needs to be set using
VIEWFS_OVERLOAD_SCHEME_KEY? I would have assumed that we use "view://" similar
to ViewFileSystem. Will we have valid use cases where fs.getScheme() needs to
match "hdfs"?
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFsOverloadScheme.java
##########
@@ -0,0 +1,175 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.viewfs;
+
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsConstants;
+import org.apache.hadoop.fs.UnsupportedFileSystemException;
+
+/******************************************************************************
Review comment:
The explanation here is a little confusing. I think it's easier to
provide an example on how it works rather than try to write about it.
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFsOverloadScheme.java
##########
@@ -0,0 +1,175 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.viewfs;
+
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsConstants;
+import org.apache.hadoop.fs.UnsupportedFileSystemException;
+
+/******************************************************************************
+ * This class is extended from the ViewFileSystem for the overloaded scheme
+ * file system. This object is the way end-user code interacts with a multiple
+ * mounted file systems transparently. Overloaded scheme based uri can be
+ * continued to use as end user interactive uri and mount links can be
+ * configured to any Hadoop compatible file system. This class maintains all
+ * the target file system instances and delegates the calls to respective
+ * target file system based on mount link mapping. Mount link configuration
+ * format and behavior is same as ViewFileSystem.
+ *****************************************************************************/
[email protected]({ "MapReduce", "HBase", "Hive" })
[email protected]
+public class ViewFsOverloadScheme extends ViewFileSystem {
+
+ public ViewFsOverloadScheme() throws IOException {
+ super();
+ }
+
+ private FsCreator fsCreator;
+ private String myScheme;
+
+ @Override
+ public String getScheme() {
+ return myScheme;
+ }
+
+ @Override
+ public void initialize(final URI theUri, final Configuration conf)
+ throws IOException {
+ superFSInit(theUri, conf);
+ setConf(conf);
+ config = conf;
+ myScheme = config.get(FsConstants.VIEWFS_OVERLOAD_SCHEME_KEY);
+ fsCreator = new FsCreator() {
+
+ /**
+ * This method is overridden because in ViewFsOverloadScheme if
+ * overloaded scheme matches with mounted target fs scheme, file system
+ * should be created without going into fs.<scheme>.impl based
+ * resolution. Otherwise it will end up into loop as target will be
+ * resolved again to ViewFsOverloadScheme as fs.<scheme>.impl points to
+ * ViewFsOverloadScheme. So, below method will initialize the
+ * fs.viewfs.overload.scheme.target.<scheme>.impl. Other schemes can
+ * follow fs.newInstance
+ */
+ @Override
+ public FileSystem createFs(URI uri, Configuration conf)
+ throws IOException {
+ if (uri.getScheme().equals(myScheme)) {
Review comment:
Currently, the override only works for one scheme. This alone can
prevent the circular dependency. However, should we consider calling
createFileSystem for all schemes that have
FS_VIEWFS_OVERLOAD_SCHEME_TARGET_FS_IMPL_PATTERN_KEY set?
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFsOverloadScheme.java
##########
@@ -0,0 +1,175 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.viewfs;
+
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsConstants;
+import org.apache.hadoop.fs.UnsupportedFileSystemException;
+
+/******************************************************************************
+ * This class is extended from the ViewFileSystem for the overloaded scheme
+ * file system. This object is the way end-user code interacts with a multiple
+ * mounted file systems transparently. Overloaded scheme based uri can be
+ * continued to use as end user interactive uri and mount links can be
+ * configured to any Hadoop compatible file system. This class maintains all
+ * the target file system instances and delegates the calls to respective
+ * target file system based on mount link mapping. Mount link configuration
+ * format and behavior is same as ViewFileSystem.
+ *****************************************************************************/
[email protected]({ "MapReduce", "HBase", "Hive" })
[email protected]
+public class ViewFsOverloadScheme extends ViewFileSystem {
+
+ public ViewFsOverloadScheme() throws IOException {
+ super();
+ }
+
+ private FsCreator fsCreator;
+ private String myScheme;
+
+ @Override
+ public String getScheme() {
+ return myScheme;
+ }
+
+ @Override
+ public void initialize(final URI theUri, final Configuration conf)
+ throws IOException {
+ superFSInit(theUri, conf);
+ setConf(conf);
+ config = conf;
+ myScheme = config.get(FsConstants.VIEWFS_OVERLOAD_SCHEME_KEY);
Review comment:
What if this is not set? No check for this currently.
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFsOverloadScheme.java
##########
@@ -0,0 +1,175 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.viewfs;
+
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsConstants;
+import org.apache.hadoop.fs.UnsupportedFileSystemException;
+
+/******************************************************************************
+ * This class is extended from the ViewFileSystem for the overloaded scheme
+ * file system. This object is the way end-user code interacts with a multiple
+ * mounted file systems transparently. Overloaded scheme based uri can be
+ * continued to use as end user interactive uri and mount links can be
+ * configured to any Hadoop compatible file system. This class maintains all
+ * the target file system instances and delegates the calls to respective
+ * target file system based on mount link mapping. Mount link configuration
+ * format and behavior is same as ViewFileSystem.
+ *****************************************************************************/
[email protected]({ "MapReduce", "HBase", "Hive" })
[email protected]
+public class ViewFsOverloadScheme extends ViewFileSystem {
+
+ public ViewFsOverloadScheme() throws IOException {
+ super();
+ }
+
+ private FsCreator fsCreator;
+ private String myScheme;
+
+ @Override
+ public String getScheme() {
+ return myScheme;
+ }
+
+ @Override
+ public void initialize(final URI theUri, final Configuration conf)
+ throws IOException {
+ superFSInit(theUri, conf);
+ setConf(conf);
+ config = conf;
+ myScheme = config.get(FsConstants.VIEWFS_OVERLOAD_SCHEME_KEY);
+ fsCreator = new FsCreator() {
+
+ /**
+ * This method is overridden because in ViewFsOverloadScheme if
+ * overloaded scheme matches with mounted target fs scheme, file system
+ * should be created without going into fs.<scheme>.impl based
+ * resolution. Otherwise it will end up into loop as target will be
Review comment:
nit: "into loop" -> "in an infinite loop as the target"
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFsOverloadScheme.java
##########
@@ -0,0 +1,175 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.viewfs;
+
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsConstants;
+import org.apache.hadoop.fs.UnsupportedFileSystemException;
+
+/******************************************************************************
+ * This class is extended from the ViewFileSystem for the overloaded scheme
+ * file system. This object is the way end-user code interacts with a multiple
+ * mounted file systems transparently. Overloaded scheme based uri can be
+ * continued to use as end user interactive uri and mount links can be
+ * configured to any Hadoop compatible file system. This class maintains all
+ * the target file system instances and delegates the calls to respective
+ * target file system based on mount link mapping. Mount link configuration
+ * format and behavior is same as ViewFileSystem.
+ *****************************************************************************/
[email protected]({ "MapReduce", "HBase", "Hive" })
[email protected]
+public class ViewFsOverloadScheme extends ViewFileSystem {
+
+ public ViewFsOverloadScheme() throws IOException {
+ super();
+ }
+
+ private FsCreator fsCreator;
+ private String myScheme;
+
+ @Override
+ public String getScheme() {
+ return myScheme;
+ }
+
+ @Override
+ public void initialize(final URI theUri, final Configuration conf)
+ throws IOException {
+ superFSInit(theUri, conf);
+ setConf(conf);
+ config = conf;
+ myScheme = config.get(FsConstants.VIEWFS_OVERLOAD_SCHEME_KEY);
+ fsCreator = new FsCreator() {
+
+ /**
+ * This method is overridden because in ViewFsOverloadScheme if
+ * overloaded scheme matches with mounted target fs scheme, file system
Review comment:
nit: "with mounted target fs scheme" -> "the scheme of the target fs"
"file system should be created" -> "the target file system .."
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFsOverloadScheme.java
##########
@@ -0,0 +1,175 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs.viewfs;
+
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsConstants;
+import org.apache.hadoop.fs.UnsupportedFileSystemException;
+
+/******************************************************************************
+ * This class is extended from the ViewFileSystem for the overloaded scheme
+ * file system. This object is the way end-user code interacts with a multiple
+ * mounted file systems transparently. Overloaded scheme based uri can be
+ * continued to use as end user interactive uri and mount links can be
+ * configured to any Hadoop compatible file system. This class maintains all
+ * the target file system instances and delegates the calls to respective
+ * target file system based on mount link mapping. Mount link configuration
+ * format and behavior is same as ViewFileSystem.
+ *****************************************************************************/
[email protected]({ "MapReduce", "HBase", "Hive" })
[email protected]
+public class ViewFsOverloadScheme extends ViewFileSystem {
+
+ public ViewFsOverloadScheme() throws IOException {
+ super();
+ }
+
+ private FsCreator fsCreator;
+ private String myScheme;
+
+ @Override
+ public String getScheme() {
+ return myScheme;
+ }
+
+ @Override
+ public void initialize(final URI theUri, final Configuration conf)
Review comment:
If we add a protected method `getFsCreator(String scheme)` in
`ViewFileSystem` and override it this class,I think `initialize` can be made
much simpler. it just needs to
`conf.setBoolean(CONFIG_VIEWFS_ENABLE_INNER_CACHE, true)` and call
`super.initialize(theUri, conf)`. Is that correct?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
