-
Notifications
You must be signed in to change notification settings - Fork 38
Another fork Lift upgrade of tests #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,25 +33,31 @@ | |
import java.net.URI; | ||
|
||
import org.apache.hadoop.conf.Configuration; | ||
import org.apache.hadoop.fs.CommonConfigurationKeys; | ||
import org.apache.hadoop.fs.FileSystem; | ||
import org.apache.hadoop.fs.FileUtil; | ||
import org.apache.hadoop.fs.FilterFileSystem; | ||
import org.apache.hadoop.fs.Path; | ||
import org.apache.hadoop.fs.permission.FsPermission; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class GlusterFileSystem extends FilterFileSystem{ | ||
|
||
protected static final Logger log=LoggerFactory.getLogger(GlusterFileSystem.class); | ||
|
||
public GlusterFileSystem(){ | ||
super(new GlusterVolume()); | ||
Version v=new Version(); | ||
log.info("Initializing GlusterFS, CRC disabled."); | ||
log.info("GIT INFO="+v); | ||
log.info("GIT_TAG="+v.getTag()); | ||
} | ||
|
||
|
||
public String getScheme() { | ||
return super.getRawFileSystem().getScheme(); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as GlusterVolume - where is getScheme() part of the FileSystem API, and who's calling it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. iirc The scheme is needed because FilterFileSystem implementation throws an exception for calls to getScheme, so in any case we dont have support for it. Specificlaly the testMkdirsWithUMask will fail without getScheme for accidental reasons (i.e. there is a little if statement in the published test which checks if scheme is s3). But in any case, getScheme should be implemented i would think... Probably we should have a more direct test of it . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where is getScheme()? i've looked in 2.2 FilterFileSystem, and FileSystem and can't find it. |
||
/** Convert a path to a File. */ | ||
public File pathToFile(Path path){ | ||
return ((GlusterVolume) fs).pathToFile(path); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,19 +26,23 @@ | |
import java.io.IOException; | ||
import java.net.URI; | ||
import java.util.Arrays; | ||
import java.util.Comparator; | ||
|
||
import org.apache.hadoop.conf.Configuration; | ||
import org.apache.hadoop.fs.BlockLocation; | ||
import org.apache.hadoop.fs.CommonConfigurationKeys; | ||
import org.apache.hadoop.fs.FileStatus; | ||
import org.apache.hadoop.fs.FileUtil; | ||
import org.apache.hadoop.fs.Path; | ||
import org.apache.hadoop.fs.RawLocalFileSystem; | ||
import org.apache.hadoop.fs.permission.FsAction; | ||
import org.apache.hadoop.fs.permission.FsPermission; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class GlusterVolume extends RawLocalFileSystem{ | ||
|
||
|
||
static final Logger log = LoggerFactory.getLogger(GlusterVolume.class); | ||
|
||
/** | ||
|
@@ -59,13 +63,26 @@ public class GlusterVolume extends RawLocalFileSystem{ | |
|
||
public GlusterVolume(){ | ||
} | ||
|
||
public String getScheme(){ | ||
return this.NAME.toString(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is getScheme() part of the API? Why is it needed? |
||
|
||
public GlusterVolume(Configuration conf){ | ||
this(); | ||
this.setConf(conf); | ||
} | ||
public URI getUri() { return NAME; } | ||
|
||
@Override | ||
public boolean mkdirs(Path f,FsPermission permission) throws IOException { | ||
// Note, since umask for unix accessed file system isn't controlled via FS_PERMISSIONS_UMASK_KEY, | ||
// there might be some odd "cascading" umasks here | ||
FsPermission umask = FsPermission.getUMask(getConf()); | ||
FsPermission masked = permission.applyUMask(umask); | ||
return super.mkdirs(f,masked); | ||
} | ||
|
||
public void setConf(Configuration conf){ | ||
log.info("Initializing gluster volume.."); | ||
super.setConf(conf); | ||
|
@@ -143,25 +160,33 @@ public Path fileToPath(File path) { | |
return new Path(NAME.toString() + path.toURI().getRawPath().substring(root.length())); | ||
} | ||
|
||
public boolean rename(Path src, Path dst) throws IOException { | ||
File dest = pathToFile(dst); | ||
|
||
/* two HCFS semantics java.io.File doesn't honor */ | ||
if(dest.exists() && dest.isFile() || !(new File(dest.getParent()).exists())) return false; | ||
|
||
if (!dest.exists() && pathToFile(src).renameTo(dest)) { | ||
return true; | ||
} | ||
return FileUtil.copy(this, src, this, dst, true, getConf()); | ||
} | ||
/** | ||
* Delete the given path to a file or directory. | ||
* @param p the path to delete | ||
* @param recursive to delete sub-directories | ||
* @return true if the file or directory and all its contents were deleted | ||
* @throws IOException if p is non-empty and recursive is false | ||
*/ | ||
@Override | ||
public boolean rename(Path src, Path dst) throws IOException { | ||
File dest = pathToFile(dst); | ||
|
||
if(dest.exists() && dest.isFile()) | ||
return false; | ||
|
||
if(! new File(pathToFile(src).toString()).exists()){ | ||
//passes ContractBaseTest: testRenameNonExistantPath | ||
//return false; | ||
//passes FsMainOperationsTest: testRenameNonExistantPath | ||
throw new FileNotFoundException(pathToFile(src)+""); | ||
} | ||
|
||
if (!dest.exists() && pathToFile(src).renameTo(dest)) { | ||
return true; | ||
} | ||
return FileUtil.copy(this, src, this, dst, true, getConf()); | ||
} | ||
|
||
/** | ||
* Delete the given path to a file or directory. | ||
* @param p the path to delete | ||
* @param recursive to delete sub-directories | ||
* @return true if the file or directory and all its contents were deleted | ||
* @throws IOException if p is non-empty and recursive is false | ||
*/ | ||
@Override | ||
public boolean delete(Path p, boolean recursive) throws IOException { | ||
File f = pathToFile(p); | ||
if(!f.exists()){ | ||
|
@@ -178,35 +203,59 @@ public boolean delete(Path p, boolean recursive) throws IOException { | |
|
||
public FileStatus[] listStatus(Path f) throws IOException { | ||
File localf = pathToFile(f); | ||
FileStatus[] results; | ||
|
||
|
||
//f is a file: returns FileStatus[]{f} | ||
if (!localf.exists()) { | ||
throw new FileNotFoundException("File " + f + " does not exist"); | ||
throw new FileNotFoundException("File at path: " + f + " does not exist"); | ||
} | ||
GlusterFileStatus gfstat=new GlusterFileStatus(localf, getDefaultBlockSize(), this) ; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why move this up here? creates an extra variable, and also instantiates an object which might not be used (if the conditional below is false). |
||
if (localf.isFile()) { | ||
return new FileStatus[] { | ||
new GlusterFileStatus(localf, getDefaultBlockSize(), this) }; | ||
return new FileStatus[] { gfstat}; | ||
} | ||
|
||
//f is a directory: returns FileStatus[] {f1, f2, f3, ... } | ||
else { | ||
//Patch for testListStatusThrowsExceptionForUnreadableDir , may need to update this after HADOOP-7352 : | ||
if(! gfstat.getPermission().getUserAction().implies(FsAction.READ)){ | ||
throw new IOException( | ||
"FileStatus indicates this is an unreadable file! Permissions=" + gfstat.getPermission().toShort() + " / Path=" + gfstat.getPath()); | ||
} | ||
|
||
FileStatus[] results; | ||
File[] names = localf.listFiles(); | ||
|
||
|
||
|
||
if (names == null) { | ||
return null; | ||
} | ||
results = new FileStatus[names.length]; | ||
int j = 0; | ||
for (int i = 0; i < names.length; i++) { | ||
try { | ||
results[j] = getFileStatus(fileToPath(names[i])); | ||
j++; | ||
} | ||
catch (FileNotFoundException e) { | ||
|
||
File[] names = localf.listFiles(); | ||
if (names == null) { | ||
return null; | ||
} | ||
results = new FileStatus[names.length]; | ||
int j = 0; | ||
for (int i = 0; i < names.length; i++) { | ||
try { | ||
results[j] = getFileStatus(fileToPath(names[i])); | ||
j++; | ||
} catch (FileNotFoundException e) { | ||
// ignore the files not found since the dir list may have have changed | ||
// since the names[] list was generated. | ||
} | ||
} | ||
if (j == names.length) { | ||
return results; | ||
// ignore the files not found since the dir list may have have changed | ||
// since the names[] list was generated. | ||
} | ||
} | ||
if(getConf().getBoolean("fs.glusterfs.list_status_sorted", false)){ | ||
Arrays.sort(results, new Comparator<FileStatus>(){ | ||
public int compare(FileStatus o1,FileStatus o2){ | ||
return o1.getPath().getName().compareTo(o2.getPath().getName()); | ||
} | ||
}); | ||
} | ||
|
||
if (j == names.length) { | ||
return results; | ||
} | ||
return Arrays.copyOf(results, j); | ||
} | ||
return Arrays.copyOf(results, j); | ||
} | ||
|
||
public FileStatus getFileStatus(Path f) throws IOException { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package org.apache.hadoop.fs.test.connector.glusterfs; | ||
|
||
import org.apache.hadoop.conf.Configuration; | ||
import org.apache.hadoop.fs.CommonConfigurationKeys; | ||
import org.apache.hadoop.fs.glusterfs.GlusterVolume; | ||
import org.apache.hadoop.fs.test.connector.HcfsTestConnector; | ||
|
||
|
@@ -13,6 +14,13 @@ public Configuration createConfiguration(){ | |
Configuration c = super.createConfiguration(); | ||
c.set("fs.glusterfs.mount",System.getProperty("GLUSTER_MOUNT")); | ||
c.set("fs.glusterfs.impl","org.apache.hadoop.fs.local.GlusterFs"); | ||
//dont apply umask, that way permissions tests for mkdir w/ 777 pass. | ||
c.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY,"000"); | ||
|
||
//So that sorted implementation of testListStatus passes if it runs. | ||
//Note that in newer FSMainOperations tests, testListStatus doesnt require. | ||
c.set("fs.glusterfs.list_status_sorted", "true"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. convention is . not _ |
||
|
||
c.set("fs.default.name","glusterfs:///"); | ||
c.setInt("io.file.buffer.size",GlusterVolume.OVERRIDE_WRITE_BUFFER_SIZE ); | ||
return c; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be removed.