Skip to content

Commit

Permalink
pnfsmanager: fix atime update regression
Browse files Browse the repository at this point in the history
Motivation:
Setting atime-gap to -1 (default value) should disable file's last access
time updates. Nevertheless, this was not the case and atime update was always
enabled.

The main reason of disabling atime update is to reduce the load on DB.

Modification:
Fix atime condition check from != -1 to >= 0 as

TimeUnit.SECONDS.toMillis(-1) return -1000.

Result:
File's last access time can be disabled as described in the documentation.

Fixes: #2390
Acked-by: Gerd Behrmann
Target: master, 2.15, 2.14, 2.13, 2.12
Require-book: no
Require-notes: yes
(cherry picked from commit 487095d)
Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
  • Loading branch information
kofemann committed May 3, 2016
1 parent 94808ce commit 5fd4a3d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
Expand Up @@ -591,6 +591,38 @@ public void testCancelUploadRecursively() throws ChimeraFsException
assertNotExists("/test");
}

@Test
public void testUpdateAtimeOnGetFileAttributes() throws ChimeraFsException {

FsInode inode = _fs.createFile("/file1");
Stat stat_before = inode.stat();
_pnfsManager.setAtimeGap(0);

PnfsGetFileAttributes message = new PnfsGetFileAttributes(new PnfsId(inode.getId()), SOME_ATTRIBUTES);
message.setUpdateAtime(true);

_pnfsManager.getFileAttributes(message);
Stat stat_after = inode.stat();

assertTrue("atime is not updated", stat_after.getATime() != stat_before.getATime());
}

@Test
public void testNoAtimeUpdateOnGetFileAttributesNegativeGap() throws ChimeraFsException {

FsInode inode = _fs.createFile("/file1");
Stat stat_before = inode.stat();
_pnfsManager.setAtimeGap(-1);

PnfsGetFileAttributes message = new PnfsGetFileAttributes(new PnfsId(inode.getId()), SOME_ATTRIBUTES);
message.setUpdateAtime(true);

_pnfsManager.getFileAttributes(message);
Stat stat_after = inode.stat();

assertTrue("atime is updated, but shouldn't", stat_after.getATime() == stat_before.getATime());
}

private void assertNotExists(String path) throws ChimeraFsException
{
try {
Expand Down
Expand Up @@ -273,7 +273,11 @@ public void setDirectoryListLimit(int limit)

@Required
public void setAtimeGap(long gap) {
_atimeGap = TimeUnit.SECONDS.toMillis(gap);
if (gap < 0) {
_atimeGap = -1;
} else {
_atimeGap = TimeUnit.SECONDS.toMillis(gap);
}
}

@Required
Expand Down Expand Up @@ -1936,7 +1940,7 @@ public void getFileAttributes(PnfsGetFileAttributes message)
checkMask(message);
checkRestriction(message, READ_METADATA);
Set<FileAttribute> requested = message.getRequestedAttributes();
if (message.getUpdateAtime() && _atimeGap != -1) {
if (message.getUpdateAtime() && _atimeGap >= 0) {
requested.add(ACCESS_TIME);
}
if(requested.contains(FileAttribute.STORAGEINFO)) {
Expand Down Expand Up @@ -1971,7 +1975,7 @@ public void getFileAttributes(PnfsGetFileAttributes message)

message.setFileAttributes(attrs);
message.setSucceeded();
if (message.getUpdateAtime() && _atimeGap != -1) {
if (message.getUpdateAtime() && _atimeGap >= 0) {
long now = System.currentTimeMillis();
if (attrs.getFileType() == FileType.REGULAR && Math.abs(now - attrs.getAccessTime()) > _atimeGap) {
FileAttributes atimeUpdateAttr = new FileAttributes();
Expand Down

0 comments on commit 5fd4a3d

Please sign in to comment.