Skip to content

Commit

Permalink
chimera: fix race condition on remove
Browse files Browse the repository at this point in the history
the remove operation does a three steps:

 1. remove entry in a directory
 2. decrease files nlink count
 3. decrease prent directory nlink count

If two threads do the same operation in the very same
moment, then step (3) done twice, while step (1) effectively
happens only once.

This patch introduces a check for result of step (1) and
does not proceeds if file is already removed.

Observer at DESY on cloud instance, where two cloud servers was running a cron
job, which happens to run the same create->remove cycle at very same moment.

The procedure to fix invalid nlink count:

UPDATE t_inodes SET inlink = (
    SELECT COUNT(*) FROM t_dirs  WHERE t_inodes.ipnfsid = t_dirs.iparent
    ) WHERE itype = 16384;

Acked-by: Paul Millar
Target: master  >>>  2.6
Require-book: no
Require-notes: yes
(cherry picked from commit fb33e16)
Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
  • Loading branch information
kofemann committed Feb 12, 2015
1 parent ceb06da commit b1aff6a
Showing 1 changed file with 29 additions and 26 deletions.
55 changes: 29 additions & 26 deletions modules/chimera/src/main/java/org/dcache/chimera/FsSqlDriver.java
Original file line number Diff line number Diff line change
Expand Up @@ -281,38 +281,40 @@ private void removeDir(Connection dbConnection, FsInode parent, FsInode inode, S
throw new DirNotEmptyHimeraFsException("directory is not empty");
}

removeEntryInParent(dbConnection, inode, ".");
removeEntryInParent(dbConnection, inode, "..");
// decrease reference count ( '.' , '..', and in parent directory ,
// and inode itself)
decNlink(dbConnection, inode, 2);
removeTag(dbConnection, inode);

removeEntryInParent(dbConnection, parent, name);
decNlink(dbConnection, parent);
setFileMTime(dbConnection, parent, 0, System.currentTimeMillis());
if (removeEntryInParent(dbConnection, parent, name)) {
removeEntryInParent(dbConnection, inode, ".");
removeEntryInParent(dbConnection, inode, "..");
// decrease reference count ( '.' , '..', and in parent directory ,
// and inode itself)
decNlink(dbConnection, inode, 2);
removeTag(dbConnection, inode);

removeInode(dbConnection, inode);
decNlink(dbConnection, parent);
setFileMTime(dbConnection, parent, 0, System.currentTimeMillis());

removeInode(dbConnection, inode);
}
}

private void removeFile(Connection dbConnection, FsInode parent, FsInode inode, String name) throws ChimeraFsException, SQLException {

boolean isLast = inode.stat().getNlink() == 1;

decNlink(dbConnection, inode);
removeEntryInParent(dbConnection, parent, name);
if (removeEntryInParent(dbConnection, parent, name)) {
decNlink(dbConnection, inode);

if (isLast) {
removeInode(dbConnection, inode);
}
if (isLast) {
removeInode(dbConnection, inode);
}

/* During bulk deletion of files in the same directory,
* updating the parent inode is often a contention point. The
* link count on the parent is updated last to reduce the time
* in which the directory inode is locked by the database.
*/
decNlink(dbConnection, parent);
setFileMTime(dbConnection, parent, 0, System.currentTimeMillis());
/* During bulk deletion of files in the same directory,
* updating the parent inode is often a contention point. The
* link count on the parent is updated last to reduce the time
* in which the directory inode is locked by the database.
*/
decNlink(dbConnection, parent);
setFileMTime(dbConnection, parent, 0, System.currentTimeMillis());
}
}

void remove(Connection dbConnection, FsInode parent, FsInode inode) throws ChimeraFsException, SQLException {
Expand Down Expand Up @@ -858,20 +860,21 @@ void removeEntryInParentByID(Connection dbConnection, FsInode parent, FsInode in
}
private static final String sqlRemoveEntryInParentByName = "DELETE FROM t_dirs WHERE iname=? AND iparent=?";

void removeEntryInParent(Connection dbConnection, FsInode parent, String name) throws SQLException {
boolean removeEntryInParent(Connection dbConnection, FsInode parent, String name) throws SQLException {
boolean removed;
PreparedStatement stRemoveFromParentByName = null; // remove entry from parent
try {

stRemoveFromParentByName = dbConnection.prepareStatement(sqlRemoveEntryInParentByName);
stRemoveFromParentByName.setString(1, name);
stRemoveFromParentByName.setString(2, parent.toString());

stRemoveFromParentByName.executeUpdate();
removed = stRemoveFromParentByName.executeUpdate() > 0;

} finally {
SqlHelper.tryToClose(stRemoveFromParentByName);
}

return removed;
}
private static final String sqlGetParentOf = "SELECT iparent FROM t_dirs WHERE ipnfsid=? AND iname != '.' and iname != '..'";

Expand Down

0 comments on commit b1aff6a

Please sign in to comment.