Skip to content

Commit

Permalink
[irods#187] Clear more caches to avoid stale file handle errors.
Browse files Browse the repository at this point in the history
  • Loading branch information
@korydraughn
korydraughn committed Oct 24, 2023
1 parent 080978c commit 0380d50
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 17 deletions.
31 changes: 22 additions & 9 deletions irods-vfs-impl/src/main/java/org/irods/nfsrods/vfs/IRODSVirtualFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,16 +131,16 @@ public class IRODSVirtualFileSystem implements VirtualFileSystem, AclCheckable

private final MutableConfiguration<String, ObjectType> objectTypeCacheConfig_; // Key: <path>
private final Cache<String, ObjectType> objectTypeCache_; // Key: <path>

private final MutableConfiguration<String, Object> permsCacheConfig_; // Key: <path>
private final Cache<String, Object> permsCache_; // Key: <path>

private final MutableConfiguration<String, UserTypeEnum> userTypeCacheConfig_; // Key: <username>
private final Cache<String, UserTypeEnum> userTypeCache_; // Key: <username>

private final MutableConfiguration<String, Object> listOpCacheConfig_; // Key: <username>#<collection>
private final Cache<String, Object> listOpCache_; // Key: <username>#<collection>

// Special paths within iRODS.
private final Path ROOT_COLLECTION;
private final Path ZONE_COLLECTION;
Expand Down Expand Up @@ -290,6 +290,10 @@ public Inode create(Inode _parent, Type _type, String _name, Subject _subject, i
}
}

listOpCache_.remove(acct.getUserName() + "#" + parentPath.toString());
statObjectCache_.remove(acct.getUserName() + "_" + parentPath.toString());
statObjectCache_.remove(acct.getUserName() + "_" + path.toString());

CollectionAndDataObjectListAndSearchAO lao = factory_.getCollectionAndDataObjectListAndSearchAO(adminAcct_);
ObjStat objStat = lao.retrieveObjectStatForPath(newFile.getAbsolutePath());

Expand Down Expand Up @@ -1123,9 +1127,10 @@ public Inode mkdir(Inode _inode, String _path, Subject _subject, int _mode) thro
try
{
Path parentPath = getPath(toInodeNumber(_inode));
Path path = parentPath.resolve(_path);

IRODSAccount acct = getCurrentIRODSUser().getAccount();
IRODSFile file = factory_.getIRODSFileFactory(acct).instanceIRODSFile(parentPath.toString(), _path);
IRODSFile file = factory_.getIRODSFileFactory(acct).instanceIRODSFile(path.toString());

file.mkdir();
file.close();
Expand All @@ -1137,6 +1142,8 @@ public Inode mkdir(Inode _inode, String _path, Subject _subject, int _mode) thro
// To get around this limitation, NFSRODS must manually clear this cache
// so that the user sees the updates.
listOpCache_.remove(acct.getUserName() + "#" + parentPath.toString());
statObjectCache_.remove(acct.getUserName() + "_" + parentPath.toString());
statObjectCache_.remove(acct.getUserName() + "_" + path.toString());

CollectionAndDataObjectListAndSearchAO lao = factory_.getCollectionAndDataObjectListAndSearchAO(adminAcct_);
ObjStat objStat = lao.retrieveObjectStatForPath(file.getAbsolutePath());
Expand Down Expand Up @@ -1191,7 +1198,8 @@ public boolean move(Inode _srcParentInode, String _srcFilename, Inode _dstParent

// Capture the existence of the destination path. This is required to properly
// update the mappings between inode numbers and paths.
final boolean unmapDstPath = dstFile.exists();
final boolean dstFileExists = dstFile.exists();
final boolean dstFileIsDataObject = (dstFileExists && dstFile.isFile());

try (AutoClosedIRODSFile ac0 = new AutoClosedIRODSFile(srcFile);
AutoClosedIRODSFile ac1 = new AutoClosedIRODSFile(dstFile))
Expand All @@ -1212,9 +1220,13 @@ else if (srcFile.isDirectory())

log_.debug("move - Updating mappings between paths and inodes ...");

// If the destination path exists in iRODS and has been mapped by the NFSRODS
// server, then that path must be unmapped before remapping to avoid an exception.
if (unmapDstPath)
// If the destination path represents a data object, unmap it. This reflects
// the fact that if both the source and destination represent files, the destination
// object will no longer exist.
//
// iRODS does not support renaming over an existing collection, so operations such
// as "mv -T" are not supported.
if (dstFileIsDataObject)
{
Long inodeNumber = inodeToPathMapper_.getInodeNumberByPath(dstPath);

Expand All @@ -1224,6 +1236,7 @@ else if (srcFile.isDirectory())
}
}

// Always map the inode number of the source path to the destination path.
inodeToPathMapper_.remap(getInodeNumber(srcPath), srcPath, dstPath);

// Because iRODS timestamps are stored in seconds, operations that trigger
Expand Down
79 changes: 71 additions & 8 deletions test/irods_tests.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,30 @@
# From within an NFSRODS mount point where you have full permission, meaning
# your iRODS home collection or a sub-collection, run the following command:
#
# $ bats test_nfsrods.bats
# bats /path/to/irods_client_nfsrods/test/irods_tests.bats
#

LS_EXECUTABLE=/bin/ls
SANDBOX=${PWD}/nfsrods_testing_sandbox
# This is run before the "first" call to "setup".
setup_file() {
export LS_EXECUTABLE=/bin/ls
export SANDBOX=${PWD}/nfsrods_testing_sandbox
}

# This is run after the "last" call to "teardown".
teardown_file() {
unset LS_EXECUTABLE
unset SANDBOX
}

# This is run before each test.
setup() {
mkdir -p ${SANDBOX}
cd ${SANDBOX}
}

# This is run after each test.
teardown() {
cd ..
cd ${SANDBOX}/..
rm -rf ${SANDBOX}
}

Expand Down Expand Up @@ -88,20 +99,21 @@ teardown() {
}

@test "listing directory with large number of entries does not trigger duplicate cookie error" {
run parallel mkdir -p ::: c{001..125}
[ $status -eq 0 ]
parallel mkdir -p ::: c{001..125}

run ${LS_EXECUTABLE}
[[ ! $(dmesg | tail | grep 'has duplicate cookie') ]]

parallel rmdir ::: c{001..125}
}

@test "listing directory with large number of entries prints all entries" {
run parallel touch ::: foo{0001..6000}
parallel touch ::: f{0001..6000}

result="$(${LS_EXECUTABLE} | wc -l)"
[ "$result" -eq 6000 ]

run parallel rm ::: foo{0001..6000}
parallel rm ::: f{0001..6000}
}

@test "create and remove directory" {
Expand Down Expand Up @@ -210,3 +222,54 @@ teardown() {
[ "$output" = "${FILENAME_COPIED}" ]
}

@test "ls does not report stale file handle error (data objects)" {
# The sequence of operations executed in this test are required to
# properly reproduce the error.
#
# DO NOT modify the behavior of this test!

local DIRECTORY="test.$(date +%s).d"
mkdir ${DIRECTORY}
rmdir ${DIRECTORY}

mkdir ${DIRECTORY}
cd ${DIRECTORY}

local FILENAME=foo
touch ${FILENAME}
rm ${FILENAME}

# THIS IS THE REAL TEST.
# Listing the contents of the collection should not result
# in an error (i.e. normally returns 2 if there's error).
${LS_EXECUTABLE}

cd ..
rmdir ${DIRECTORY}
}

@test "ls does not report stale file handle error (collections)" {
# The sequence of operations executed in this test are required to
# properly reproduce the error.
#
# DO NOT modify the behavior of this test!

local DIRECTORY="test.$(date +%s).d"
mkdir ${DIRECTORY}
rmdir ${DIRECTORY}

mkdir ${DIRECTORY}
cd ${DIRECTORY}

local NEW_DIRECTORY=foo.d
mkdir ${NEW_DIRECTORY}
rmdir ${NEW_DIRECTORY}

# THIS IS THE REAL TEST.
# Listing the contents of the collection should not result
# in an error (i.e. normally returns 2 if there's error).
${LS_EXECUTABLE}

cd ..
rmdir ${DIRECTORY}
}

0 comments on commit 0380d50

Please sign in to comment.