Skip to content

Commit

Permalink
chimera: Revert "chimera: update FsSqlDriver#inodeOf to throw excepti…
Browse files Browse the repository at this point in the history 
…on if file not found"

Motivation:

Issue #7469 reports that
xrood copy to existing fie path with overwrite flag ends up removing
destination but no fie replaced.

Modification:

Just by bisecting this patch has been identified as culprit:
2fb9170.
This patch reverts commit 2fb9170

Result:

copy to w/ overwrite flag work properly

Target: trunk
Request: 9.2
Acked-by: Lea Morschel, Paul Millar
Patch: https://rb.dcache.org/r/14187/
Require-notes: yes
  • Loading branch information
@DmitryLitvintsev
DmitryLitvintsev committed Dec 19, 2023
1 parent 38d2cae commit 2ac2180
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 42 deletions.
36 changes: 16 additions & 20 deletions modules/chimera/src/main/java/org/dcache/chimera/FsSqlDriver.java
View file
Expand Up @@ -511,24 +511,21 @@ boolean rename(FsInode inode, FsInode srcDir, String source, FsInode destDir, St
*
* @param parent
* @param name
* @throws FileNotFoundChimeraFsException if name does not exist in parent path
* @return the inode for the named file in the parent directory.
* @return null if path is not found
*/
FsInode inodeOf(FsInode parent, String name, StatCacheOption stat)
throws FileNotFoundChimeraFsException {
FsInode inodeOf(FsInode parent, String name, StatCacheOption stat) {
switch (name) {
case ".":
return parent.isDirectory() ? parent : null;
case "..":
if (!parent.isDirectory()) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, name);
return null;
}
FsInode dir = parent.getParent();
return (dir == null) ? parent : dir;
default:
FsInode child;
if (stat == STAT) {
child = _jdbc.query(
return _jdbc.query(
"SELECT c.* FROM t_dirs d JOIN t_inodes c ON d.ichild = c.inumber " +
"WHERE d.iparent = ? AND d.iname = ?",
ps -> {
Expand All @@ -538,18 +535,14 @@ FsInode inodeOf(FsInode parent, String name, StatCacheOption stat)
rs -> rs.next() ? new FsInode(parent.getFs(), rs.getLong("inumber"),
FsInodeType.INODE, 0, toStat(rs)) : null);
} else {
child = _jdbc.query("SELECT ichild FROM t_dirs WHERE iparent=? AND iname=?",
return _jdbc.query("SELECT ichild FROM t_dirs WHERE iparent=? AND iname=?",
ps -> {
ps.setLong(1, parent.ino());
ps.setString(2, name);
},
rs -> rs.next() ? new FsInode(parent.getFs(), rs.getLong("ichild"))
: null);
}
if (child == null) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, name);
}
return child;
}
}

Expand Down Expand Up @@ -1620,10 +1613,13 @@ FsInode path2inode(FsInode root, String path) throws ChimeraFsException {
*/
for (int i = pathElemts.size(); i > 0; i--) {
String f = pathElemts.get(i - 1);
try {
inode = inodeOf(parentInode, f, STAT);
} catch (FileNotFoundChimeraFsException e) {
return null;
inode = inodeOf(parentInode, f, STAT);

if (inode == null) {
/*
* element not found stop walking
*/
break;
}

/*
Expand All @@ -1650,7 +1646,7 @@ FsInode path2inode(FsInode root, String path) throws ChimeraFsException {
*
* @param root staring point
* @param path
* @return inode or an empty collection if path does not exist.
* @return inode or null if path does not exist.
*/
List<FsInode> path2inodes(FsInode root, String path) throws ChimeraFsException {
File pathFile = new File(path);
Expand All @@ -1675,9 +1671,9 @@ List<FsInode> path2inodes(FsInode root, String path) throws ChimeraFsException {
/* Path elements are in reverse order.
*/
for (String f : Lists.reverse(pathElements)) {
try {
inode = inodeOf(parentInode, f, STAT);
} catch (FileNotFoundChimeraFsException e) {
inode = inodeOf(parentInode, f, STAT);

if (inode == null) {
return Collections.emptyList();
}

Expand Down
67 changes: 45 additions & 22 deletions modules/chimera/src/main/java/org/dcache/chimera/JdbcFs.java
View file
Expand Up @@ -423,6 +423,9 @@ public FsInode createFile(FsInode parent, String name, int owner, int group, int
int level = Integer.parseInt(cmd[1]);
return inTransaction(status -> {
FsInode useInode = _sqlDriver.inodeOf(parent, cmd[2], STAT);
if (useInode == null) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, cmd[2]);
}
try {
Stat stat = useInode.statCache();
return _sqlDriver.createLevel(useInode, stat.getUid(),
Expand Down Expand Up @@ -559,7 +562,7 @@ public void remove(String path) throws ChimeraFsException {
FsInode parent = path2inode(parentPath);
String name = filePath.getName();
FsInode inode = _sqlDriver.inodeOf(parent, name, STAT);
if (!_sqlDriver.remove(parent, name, inode)) {
if (inode == null || !_sqlDriver.remove(parent, name, inode)) {
throw FileNotFoundChimeraFsException.ofPath(path);
}
return null;
Expand Down Expand Up @@ -796,6 +799,9 @@ public FsInode inodeOf(FsInode parent, String name, StatCacheOption cacheOption)
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, name);
}
FsInode inode = _sqlDriver.inodeOf(parent, cmd[1], NO_STAT);
if (inode == null) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, cmd[1]);
}
return new FsInode_ID(this, inode.ino());
}

Expand All @@ -808,6 +814,9 @@ public FsInode inodeOf(FsInode parent, String name, StatCacheOption cacheOption)
int level = Integer.parseInt(cmd[1]);

FsInode inode = _sqlDriver.inodeOf(parent, cmd[2], NO_STAT);
if (inode == null) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, cmd[2]);
}
if (level <= LEVELS_NUMBER) {
stat(inode, level);
return new FsInode(this, inode.ino(), level);
Expand Down Expand Up @@ -908,6 +917,9 @@ public FsInode inodeOf(FsInode parent, String name, StatCacheOption cacheOption)
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, name);
}
FsInode inode = _sqlDriver.inodeOf(parent, cmd[1], NO_STAT);
if (inode == null) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, cmd[1]);
}
return new FsInode_SURI(this, inode.ino());
}

Expand Down Expand Up @@ -948,6 +960,9 @@ public FsInode inodeOf(FsInode parent, String name, StatCacheOption cacheOption)
}

FsInode inode = _sqlDriver.inodeOf(parent, cmd[1], NO_STAT);
if (inode == null) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, cmd[1]);
}
switch (cmd[2]) {
case "locality":
return new FsInode_PLOC(this, inode.ino());
Expand All @@ -970,7 +985,11 @@ public FsInode inodeOf(FsInode parent, String name, StatCacheOption cacheOption)
if (cmd.length != 2) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, name);
}
return _sqlDriver.inodeOf(getWormID(), cmd[1], NO_STAT);
FsInode inode = _sqlDriver.inodeOf(getWormID(), cmd[1], NO_STAT);
if (inode == null) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, cmd[1]);
}
return inode;
}

if (name.startsWith(".(fset)(")) {
Expand All @@ -980,6 +999,10 @@ public FsInode inodeOf(FsInode parent, String name, StatCacheOption cacheOption)
}

FsInode inode = _sqlDriver.inodeOf(parent, cmd[1], NO_STAT);
if (inode == null) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, cmd[1]);
}

String[] args = new String[cmd.length - 2];
System.arraycopy(cmd, 2, args, 0, args.length);

Expand All @@ -988,6 +1011,9 @@ public FsInode inodeOf(FsInode parent, String name, StatCacheOption cacheOption)
}

FsInode inode = _sqlDriver.inodeOf(parent, name, cacheOption);
if (inode == null) {
throw FileNotFoundChimeraFsException.ofFileInDirectory(parent, name);
}
fillIdCaches(inode);
inode.setParent(parent);
return inode;
Expand Down Expand Up @@ -1130,32 +1156,29 @@ public boolean rename(FsInode inode, FsInode srcDir, String source, FsInode dest
throw new NotDirChimeraException(destDir);
}

FsInode destInode;
try {
destInode = _sqlDriver.inodeOf(destDir, dest, STAT);
} catch (FileNotFoundChimeraFsException e) {
if (!_sqlDriver.rename(inode, srcDir, source, destDir, dest)) {
throw FileNotFoundChimeraFsException.ofPath(source);
FsInode destInode = _sqlDriver.inodeOf(destDir, dest, STAT);

if (destInode != null) {
if (destInode.equals(inode)) {
// according to POSIX, we are done
return false;
}
return true;
}

if (destInode.equals(inode)) {
// according to POSIX, we are done
return false;
}
/* Renaming into existing is only allowed for the same type of entry.
*/
if (inode.isDirectory() != destInode.isDirectory()) {
throw new FileExistsChimeraFsException(dest);
}

/* Renaming into existing is only allowed for the same type of entry.
*/
if (inode.isDirectory() != destInode.isDirectory()) {
throw new FileExistsChimeraFsException(dest);
if (!_sqlDriver.remove(destDir, dest, destInode)) {
// Concurrent modification - retry
return rename(inode, srcDir, source, destDir, dest);
}
}

if (!_sqlDriver.remove(destDir, dest, destInode)) {
// Concurrent modification - retry
return rename(inode, srcDir, source, destDir, dest);
if (!_sqlDriver.rename(inode, srcDir, source, destDir, dest)) {
throw FileNotFoundChimeraFsException.ofPath(source);
}

return true;
});
}
Expand Down

0 comments on commit 2ac2180

Please sign in to comment.