Skip to content

Commit

Permalink
dcache-xroot: improve efficiency of stat list (ls -l)
Browse files Browse the repository at this point in the history
Motivation:

The xroot ls command has some unnecessary overhead.
First, it does not need to query file locality;
second, it does not need to know file state in
case of POSC.

It would seem proper for those attributes to be
checked by a client by issuing an individual
stat on the file.

Modification:

Fork the `getFileStatus` functionality so
that stat listing (i.e., `ls -l`) is separate
from a full stat request; in the former case,
`STORAGEINFO` is not requested and a call
to the `PoolMonitor` for file locality is
avoided.

Result:

`ls -l` returns in approximately the
same amount of time as a non-stat listing
`ls`.

Target: master
Request: 9.1
Request: 9.0
Request: 8.2
Patch: https://rb.dcache.org/r/14055/
Requires-notes: yes
Acked-by: Tigran
  • Loading branch information
alrossi authored and mksahakyan committed Aug 15, 2023
1 parent 2a9e49a commit 1af82e7
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 5 deletions.
Expand Up @@ -1047,8 +1047,18 @@ public void messageArrived(PnfsListDirectoryMessage msg) {
}
}

private int getFileStatusFlags(Subject subject, Restriction restriction,
private int getFileStatusFlagsForListing(Subject subject, Restriction restriction,
FsPath path, FileAttributes attributes) {
return getFileStatusFlags(subject, restriction, path, attributes, true);
}

private int getFileStatusFlags(Subject subject, Restriction restriction,
FsPath path, FileAttributes attributes ) {
return getFileStatusFlags(subject, restriction, path, attributes, false);
}

private int getFileStatusFlags(Subject subject, Restriction restriction,
FsPath path, FileAttributes attributes, boolean skipPoscCheck) {
int flags = 0;
switch (attributes.getFileType()) {
case DIR:
Expand Down Expand Up @@ -1088,7 +1098,7 @@ private int getFileStatusFlags(Subject subject, Restriction restriction,
if (canReadFile) {
flags |= kXR_readable;
}
if (attributes.getStorageInfo().isCreatedOnly()) {
if (!skipPoscCheck && attributes.getStorageInfo().isCreatedOnly()) {
flags |= kXR_poscpend;
}
break;
Expand Down Expand Up @@ -1119,6 +1129,12 @@ public FileStatus getFileStatus(FsPath fullPath, Subject subject,
return getFileStatus(subject, restriction, fullPath, clientHost, attributes);
}

public EnumSet<FileAttribute> getRequiredAttributesForFileStatusList() {
EnumSet<FileAttribute> requestedAttributes = EnumSet.of(TYPE, SIZE, MODIFICATION_TIME);
requestedAttributes.addAll(_pdp.getRequiredAttributes());
return requestedAttributes;
}

public EnumSet<FileAttribute> getRequiredAttributesForFileStatus() {
EnumSet<FileAttribute> requestedAttributes = EnumSet.of(TYPE, SIZE, MODIFICATION_TIME,
STORAGEINFO);
Expand All @@ -1127,6 +1143,13 @@ public EnumSet<FileAttribute> getRequiredAttributesForFileStatus() {
return requestedAttributes;
}

public FileStatus getFileStatusForListing(Subject subject, Restriction restriction, FsPath fullPath,
FileAttributes attributes) {
int flags = getFileStatusFlagsForListing(subject, restriction, fullPath, attributes);
return new FileStatus(0, attributes.getSizeIfPresent().orElse(0L), flags,
attributes.getModificationTime() / 1000);
}

public FileStatus getFileStatus(Subject subject, Restriction restriction,
FsPath fullPath, String clientHost, FileAttributes attributes) {
int flags = getFileStatusFlags(subject, restriction, fullPath, attributes);
Expand Down
Expand Up @@ -1093,7 +1093,7 @@ protected XrootdResponse<DirListRequest> doOnDirList(ChannelHandlerContext ctx,
if (request.isDirectoryStat()) {
_door.listPath(fullListPath, subject, restriction,
new StatListCallback(request, subject, restriction, fullListPath, ctx),
_door.getRequiredAttributesForFileStatus());
_door.getRequiredAttributesForFileStatusList());
} else {
_door.listPath(fullListPath, subject, restriction,
new ListCallback(request, ctx),
Expand Down Expand Up @@ -1329,10 +1329,10 @@ public StatListCallback(DirListRequest request,
@Override
public void success(PnfsListDirectoryMessage message) {
message.getEntries().stream().forEach(
e -> _response.add(e.getName(), _door.getFileStatus(_subject,
e -> _response.add(e.getName(), _door.getFileStatusForListing(_subject,
_restriction,
_dirPath.child(e.getName()),
_client, e.getFileAttributes())));
e.getFileAttributes())));
if (message.isFinal()) {
respond(_context, _response.buildFinal());
} else {
Expand Down

0 comments on commit 1af82e7

Please sign in to comment.