From a29d8f4f084c0019e3dfde21be06a0dffaf71c20 Mon Sep 17 00:00:00 2001
From: Albert Louis Rossi <arossi@fnal.gov>
Date: Mon, 14 Aug 2023 08:41:09 -0500
Subject: [PATCH] dcache-xroot:  improve efficiency of stat list (ls -l)

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
---
 .../org/dcache/xrootd/door/XrootdDoor.java    | 27 +++++++++++++++++--
 .../xrootd/door/XrootdRedirectHandler.java    |  6 ++---
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdDoor.java b/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdDoor.java
index 31a70400010..f1cd19a23c3 100644
--- a/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdDoor.java
+++ b/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdDoor.java
@@ -1053,8 +1053,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:
@@ -1094,7 +1104,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;
@@ -1125,6 +1135,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);
@@ -1133,6 +1149,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);
diff --git a/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdRedirectHandler.java b/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdRedirectHandler.java
index c24b280dfd2..02cd2e4612e 100644
--- a/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdRedirectHandler.java
+++ b/modules/dcache-xrootd/src/main/java/org/dcache/xrootd/door/XrootdRedirectHandler.java
@@ -1106,7 +1106,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),
@@ -1342,10 +1342,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 {