MGM: GroupBalancer/Drainer: make file filter configurable

Make the function that selects files allow a filter argument which defaults to
Null so that GroupBalancer can filter out proc files whereas the GroupDrainer
needs to not skip these.

Fixes: EOS-6382
Signed-off-by: Abhishek Lekshmanan <abhishek.lekshmanan@cern.ch>

Author: theanalyst

mgm/GroupBalancer.cc

@@ -41,13 +41,14 @@ EOSMGMNAMESPACE_BEGIN
 using group_balancer::BalancerEngineT;
 using group_balancer::group_size_map;
 using group_balancer::eosGroupsInfoFetcher;
-
+using group_balancer::PrefixFilter;
 
 //-------------------------------------------------------------------------------
 // GroupBalancer constructor
 //-------------------------------------------------------------------------------
 GroupBalancer::GroupBalancer(const char* spacename)
-  : mSpaceName(spacename), mLastCheck(0)
+  : mSpaceName(spacename), mLastCheck(0),
+    mProcFilter(PrefixFilter(gOFS->MgmProcPath.c_str()))
 {
   mEngine.reset(group_balancer::make_balancer_engine(BalancerEngineT::stddev));
   mThread.reset(&GroupBalancer::GroupBalance, this);
@@ -216,7 +217,8 @@ GroupBalancer::chooseFileFromGroup(FsGroup* from_group, FsGroup* to_group,
 
     auto filename = group_balancer::getFileProcTransferNameAndSize(fid,
                     to_group->mName,
-                    &filesize);
+                    &filesize,
+                    mProcFilter);
 
     if (filename.empty() ||
         (mCfg.mMinFileSize > filesize) ||

mgm/GroupBalancer.hh

@@ -33,6 +33,7 @@
 #include <map>
 #include <unordered_set>
 #include "mgm/groupbalancer/BalancerEngineTypes.hh"
+#include "mgm/groupbalancer/ConverterUtils.hh"
 
 EOSMGMNAMESPACE_BEGIN
 
@@ -158,7 +159,7 @@ private:
   //! Scheduled transfers (maps fid to path in proc)
   std::map<eos::common::FileId::fileid_t, std::string> mTransfers;
   group_balancer::engine_conf_t mEngineConf;
-
+  group_balancer::SkipFileFn mProcFilter;
 
   //----------------------------------------------------------------------------
   //! Chooses a random file ID from a random filesystem in the given group

mgm/GroupDrainer.cc

@@ -313,7 +313,8 @@ GroupDrainer::scheduleTransfer(eos::common::FileId::fileid_t fid,
   }
 
   uint64_t filesz;
-  auto conv_tag = getFileProcTransferNameAndSize(fid, tgt_grp, &filesz);
+  auto conv_tag = getFileProcTransferNameAndSize(fid, tgt_grp, &filesz,
+                                                 group_balancer::NullFilter);
 
   if (conv_tag.empty()) {
     eos_err("msg=\"Possibly failed proc file found\" fid=%08llx", fid);

mgm/groupbalancer/ConverterUtils.cc

@@ -1,8 +1,6 @@
-//
-// Created by abhi on 4/26/22.
-//
-
 #include "ConverterUtils.hh"
+
+#include "common/StringUtils.hh"
 #include "common/Logging.hh"
 #include "common/LayoutId.hh"
 #include "namespace/Prefetcher.hh"
@@ -12,10 +10,14 @@
 
 namespace eos::mgm::group_balancer
 {
+bool PrefixFilter::operator()(std::string_view path) {
+    return eos::common::startsWith(path, prefix);
+}
 
 std::string
 getFileProcTransferNameAndSize(eos::common::FileId::fileid_t fid,
-                               const std::string& target_group, uint64_t* size)
+                               const std::string& target_group, uint64_t* size,
+                               const SkipFileFn& skip_file_fn)
 {
   char fileName[1024];
   std::shared_ptr<eos::IFileMD> fmd;
@@ -34,19 +36,16 @@ getFileProcTransferNameAndSize(eos::common::FileId::fileid_t fid,
         return std::string("");
       }
 
-      if (size) {
-        *size = fmd->getSize();
+      if (skip_file_fn && skip_file_fn(gOFS->eosView->getUri(fmd.get()))) {
+        return std::string("");
       }
 
-      XrdOucString fileURI = gOFS->eosView->getUri(fmd.get()).c_str();
-
-      if (fileURI.beginswith(gOFS->MgmProcPath.c_str())) {
-        // don't touch files in any ../proc/ directory
-        return std::string("");
+      if (size) {
+        *size = fmd->getSize();
       }
 
-      eos_static_debug("msg=\"found file for transfering\" file=\"%s\"",
-                       fileURI.c_str());
+      eos_static_debug("msg=\"found file for transfering\" fid=\"%08llx\"",
+                       fileid);
     } catch (eos::MDException& e) {
       eos_static_debug("msg=\"exception\" ec=%d emsg=\"%s\"\n", e.getErrno(),
                        e.getMessage().str().c_str());

mgm/groupbalancer/ConverterUtils.hh

@@ -24,24 +24,39 @@
 #ifndef EOS_CONVERTERUTILS_HH
 #define EOS_CONVERTERUTILS_HH
 
+#include <functional>
+#include <string_view>
+
 #include "common/FileId.hh"
 #include <string>
 
 namespace eos::mgm::group_balancer
 {
+
+using SkipFileFn = std::function<bool(std::string_view)>;
+inline const SkipFileFn NullFilter = {};
+
+struct PrefixFilter {
+    bool operator()(std::string_view path);
+    std::string prefix;
+    PrefixFilter(std::string_view _prefix): prefix(_prefix) {}
+};
+
 //----------------------------------------------------------------------------
 //! Produces a file conversion path to be placed in the proc directory taking
 //! into account the given group and also returns its size
 //!
 //! @param fid the file ID
 //! @param target_group the group to which the file will be transferred
 //! @param size return address for the size of the file
+//! @param skip_file_fn function to skip files matching filter
+//!          defaults to NullFilter, which means no files will be skipped
 //!
 //! @return name of the proc transfer file
 //----------------------------------------------------------------------------
 std::string
 getFileProcTransferNameAndSize(eos::common::FileId::fileid_t fid,
-                               const std::string& target_group, uint64_t* size);
-
+                               const std::string& target_group, uint64_t* size,
+                               const SkipFileFn& skip_file_fn = NullFilter);
 } // eos::mgm::group_balancer
 #endif // EOS_CONVERTERUTILS_HH

unit_tests/mgm/groupbalancer/GroupBalancerUtilsTests.cc

@@ -1,11 +1,9 @@
 #include "gtest/gtest.h"
 #include "mgm/groupbalancer/BalancerEngineUtils.hh"
+#include "mgm/groupbalancer/ConverterUtils.hh"
 
 using namespace eos::mgm::group_balancer;
 
-
-
-
 TEST(GroupBalancerUtils, Avg)
 {
   EXPECT_DOUBLE_EQ(0, calculateAvg({}));
@@ -84,3 +82,28 @@ TEST(GroupBalancerUtils, extract_commalist_value)
   EXPECT_EQ(empty,
             extract_commalist_value(conf, "some key"));
 }
+
+// a function that behaves like the skipFiles call in
+// getProcTransferNameAndSize
+std::string fakeSkipFile(const SkipFileFn& skip_file_fn,
+                         std::string_view path)
+{
+  if (skip_file_fn && skip_file_fn(path)) {
+    return std::string("");
+  }
+  return std::string(path);
+}
+
+TEST(GroupBalancerUtils, SkipFilesNullFilter)
+{
+  EXPECT_FALSE(NullFilter);
+  EXPECT_EQ("/proc/foo",fakeSkipFile(NullFilter, "/proc/foo"));
+  EXPECT_EQ("/00001/bar",fakeSkipFile(NullFilter, "/00001/bar"));
+}
+
+TEST(GroupBalancerUtils, SkipFiles)
+{
+  PrefixFilter procFilter{"/proc/"};
+  EXPECT_EQ("", fakeSkipFile(procFilter, "/proc/foo"));
+  EXPECT_EQ("/000001/bar",fakeSkipFile(procFilter, "/000001/bar"));
+}