[ntuple] several fixes to the DAOS backend

tree/ntuple/inc/ROOT/RDaos.hxx

@@ -97,7 +97,6 @@ public:
       ObjClassId(daos_oclass_id_t cid) : fCid(cid) {}
       ObjClassId(const std::string &name) : fCid(daos_oclass_name2id(name.data())) {}
 
-      bool IsUnknown() const { return fCid == OC_UNKNOWN; }
       std::string ToString() const;
 
       /// This limit is currently not defined in any header and any call to
@@ -140,9 +139,9 @@ public:
    };
 
    RDaosObject() = delete;
-   /// Provides low-level access to an object. If `cid` is OC_UNKNOWN, the user is responsible for
-   /// calling `daos_obj_generate_oid()` to fill the reserved bits in `oid` before calling this constructor.
-   RDaosObject(RDaosContainer &container, daos_obj_id_t oid, ObjClassId cid = OC_UNKNOWN);
+   /// Provides low-level access to an object. The oid is assumed to have set only the user-editable bits.
+   /// DAOS will fill in the remaining, system bits using the given object class.
+   RDaosObject(RDaosContainer &container, daos_obj_id_t oid, ObjClassId cid);
    ~RDaosObject();
 
    int Fetch(FetchUpdateArgs &args);
@@ -235,8 +234,8 @@ private:
    daos_handle_t fContainerHandle{};
    uuid_t fContainerUuid{};
    std::string fContainerLabel{};
-   std::shared_ptr<RDaosPool> fPool;
-   ObjClassId_t fDefaultObjectClass{OC_SX};
+   std::unique_ptr<RDaosPool> fPool;
+   ObjClassId_t fDefaultObjectClass{OC_UNKNOWN};
 
    /**
      \brief Perform a vector read/write operation on different objects.
@@ -249,7 +248,7 @@ private:
                        int (RDaosObject::*fn)(RDaosObject::FetchUpdateArgs &));
 
 public:
-   RDaosContainer(std::shared_ptr<RDaosPool> pool, std::string_view containerId, bool create = false);
+   RDaosContainer(std::unique_ptr<RDaosPool> pool, std::string_view containerId, bool create = false);
    ~RDaosContainer();
 
    ObjClassId_t GetDefaultObjectClass() const { return fDefaultObjectClass; }

tree/ntuple/inc/ROOT/RNTupleWriteOptionsDaos.hxx

@@ -33,7 +33,7 @@ namespace Experimental {
 */
 // clang-format on
 class RNTupleWriteOptionsDaos : public ROOT::RNTupleWriteOptions {
-   std::string fObjectClass{"SX"};
+   std::string fObjectClass{"UNKNOWN"};
 
 public:
    ~RNTupleWriteOptionsDaos() override = default;

tree/ntuple/inc/ROOT/RPageStorage.hxx

@@ -751,7 +751,10 @@ protected:
    virtual void LoadStructureImpl() = 0;
    /// `LoadStructureImpl()` has been called before `AttachImpl()` is called
    virtual ROOT::RNTupleDescriptor AttachImpl() = 0;
-   /// Returns a new, unattached page source for the same data set
+   /// Returns a new, unattached page source for the same data set. If the origin data source was attached, the
+   /// base class Clone() method will as an optimization not call LoadStructureImpl() and AttachImpl() again
+   /// but simply clone the existing descriptor. This means that CloneImpl(), for an attached page source, needs to
+   /// redo any operation other than loading the descriptor that may be done in LoadStructureImpl() and AttachImpl().
    virtual std::unique_ptr<RPageSource> CloneImpl() const = 0;
    // Only called if a task scheduler is set. No-op be default.
    virtual void UnzipClusterImpl(ROOT::Internal::RCluster *cluster);

tree/ntuple/src/RDaos.cxx

@@ -107,10 +107,8 @@ daos_event_t *ROOT::Experimental::Internal::RDaosObject::FetchUpdateArgs::GetEve
 
 ROOT::Experimental::Internal::RDaosObject::RDaosObject(RDaosContainer &container, daos_obj_id_t oid, ObjClassId cid)
 {
-   if (!cid.IsUnknown())
-      daos_obj_generate_oid(container.fContainerHandle, &oid, DAOS_OT_MULTI_UINT64, cid.fCid,
-                            DAOS_OCH_RDD_DEF | DAOS_OCH_SHD_DEF, 0);
-
+   daos_obj_generate_oid(container.fContainerHandle, &oid, DAOS_OT_MULTI_UINT64, cid.fCid,
+                         DAOS_OCH_RDD_DEF | DAOS_OCH_SHD_DEF, 0);
    if (int err = daos_obj_open(container.fContainerHandle, oid, DAOS_OO_RW, &fObjectHandle, nullptr))
       throw RException(R__FAIL("daos_obj_open: error: " + std::string(d_errstr(err))));
 }
@@ -171,9 +169,9 @@ int ROOT::Experimental::Internal::RDaosEventQueue::WaitOnParentBarrier(daos_even
 
 ////////////////////////////////////////////////////////////////////////////////
 
-ROOT::Experimental::Internal::RDaosContainer::RDaosContainer(std::shared_ptr<RDaosPool> pool,
+ROOT::Experimental::Internal::RDaosContainer::RDaosContainer(std::unique_ptr<RDaosPool> pool,
                                                              std::string_view containerId, bool create)
-   : fPool(pool)
+   : fPool(std::move(pool))
 {
    daos_cont_info_t containerInfo{};
 

tree/ntuple/src/RPageStorageDaos.cxx

@@ -56,9 +56,8 @@ struct RDaosKey {
    AttributeKey_t fAkey;
 };
 
-/// \brief Pre-defined keys for object store. `kDistributionKeyDefault` is the distribution key for metadata and
-/// pagelist values; optionally it can be used for ntuple pages (if under the `kOidPerPage` mapping strategy).
-/// `kAttributeKeyDefault` is the attribute key for ntuple pages under `kOidPerPage`.
+/// \brief Pre-defined keys for object store. `kDistributionKeyDefault` is the distribution key for all objects,
+/// `kAttributeKeyDefault` is the attribute key for all objects but anchor, header, footer.
 /// `kAttributeKey{Anchor,Header,Footer}` are the respective attribute keys for anchor/header/footer metadata elements.
 static constexpr DistributionKey_t kDistributionKeyDefault = 0x5a3c69f0cafe4a11;
 static constexpr AttributeKey_t kAttributeKeyDefault = 0x4243544b53444229;
@@ -70,7 +69,11 @@ static constexpr AttributeKey_t kAttributeKeyFooter = 0x4243544b5344422c;
 static constexpr decltype(daos_obj_id_t::lo) kOidLowMetadata = -1;
 static constexpr decltype(daos_obj_id_t::lo) kOidLowPageList = -2;
 
-static constexpr daos_oclass_id_t kCidMetadata = OC_SX;
+/// Because the object class becomes part of the object ID (encoded in the system-reserved 32 bits), we have to
+/// hard-code the object class for the anchor.  Otherwise, we would need ask the user to specify the correct object
+/// class in the RNTupleReadOptions when trying to open a previously written data set, which is not really acceptable.
+/// The object class set in the RNTupleWriteOptions thus applies to all objects except the anchor.
+static constexpr daos_oclass_id_t kCidAnchor = OC_UNKNOWN;
 
 RDaosKey GetPageDaosKey(ROOT::Experimental::Internal::ntuple_index_t ntplId, long unsigned pageCount)
 {
@@ -139,7 +142,7 @@ struct RDaosContainerNTupleLocator {
 
       buffer = MakeUninitArray<unsigned char>(anchorSize);
       if ((err = cont.ReadSingleAkey(buffer.get(), anchorSize, oidMetadata, kDistributionKeyDefault,
-                                     kAttributeKeyAnchor, kCidMetadata))) {
+                                     kAttributeKeyAnchor, kCidAnchor))) {
          return err;
       }
 
@@ -238,15 +241,12 @@ void ROOT::Experimental::Internal::RPageSinkDaos::InitImpl(unsigned char *serial
 {
    auto opts = dynamic_cast<RNTupleWriteOptionsDaos *>(fOptions.get());
    fNTupleAnchor.fObjClass = opts ? opts->GetObjectClass() : RNTupleWriteOptionsDaos().GetObjectClass();
-   auto oclass = RDaosObject::ObjClassId(fNTupleAnchor.fObjClass);
-   if (oclass.IsUnknown())
-      throw ROOT::RException(R__FAIL("Unknown object class " + fNTupleAnchor.fObjClass));
 
    auto args = ParseDaosURI(fURI);
-   auto pool = std::make_shared<RDaosPool>(args.fPoolLabel);
+   auto pool = std::make_unique<RDaosPool>(args.fPoolLabel);
 
-   fDaosContainer = std::make_unique<RDaosContainer>(pool, args.fContainerLabel, /*create =*/true);
-   fDaosContainer->SetDefaultObjectClass(oclass);
+   fDaosContainer = std::make_unique<RDaosContainer>(std::move(pool), args.fContainerLabel, /*create =*/true);
+   fDaosContainer->SetDefaultObjectClass(fNTupleAnchor.fObjClass);
 
    auto [locator, _] = RDaosContainerNTupleLocator::LocateNTuple(*fDaosContainer, fNTupleName);
    fNTupleIndex = locator.GetIndex();
@@ -357,10 +357,14 @@ ROOT::Experimental::Internal::RPageSinkDaos::CommitClusterGroupImpl(unsigned cha
       RNTupleCompressor::Zip(serializedPageList, length, GetWriteOptions().GetCompression(), bufPageListZip.get());
 
    auto offsetData = fClusterGroupId.fetch_add(1);
+   // clang-format off
    fDaosContainer->WriteSingleAkey(
-      bufPageListZip.get(), szPageListZip,
-      daos_obj_id_t{kOidLowPageList, static_cast<decltype(daos_obj_id_t::hi)>(fNTupleIndex)}, kDistributionKeyDefault,
-      offsetData, kCidMetadata);
+      bufPageListZip.get(),
+      szPageListZip,
+      daos_obj_id_t{kOidLowPageList, static_cast<decltype(daos_obj_id_t::hi)>(fNTupleIndex)},
+      kDistributionKeyDefault,
+      offsetData);
+   // clang-format on
    RNTupleLocator result;
    result.SetType(RNTupleLocator::kTypeObject64);
    result.SetNBytesOnStorage(szPageListZip);
@@ -386,7 +390,7 @@ void ROOT::Experimental::Internal::RPageSinkDaos::WriteNTupleHeader(const void *
 {
    fDaosContainer->WriteSingleAkey(
       data, nbytes, daos_obj_id_t{kOidLowMetadata, static_cast<decltype(daos_obj_id_t::hi)>(fNTupleIndex)},
-      kDistributionKeyDefault, kAttributeKeyHeader, kCidMetadata);
+      kDistributionKeyDefault, kAttributeKeyHeader);
    fNTupleAnchor.fLenHeader = lenHeader;
    fNTupleAnchor.fNBytesHeader = nbytes;
 }
@@ -395,7 +399,7 @@ void ROOT::Experimental::Internal::RPageSinkDaos::WriteNTupleFooter(const void *
 {
    fDaosContainer->WriteSingleAkey(
       data, nbytes, daos_obj_id_t{kOidLowMetadata, static_cast<decltype(daos_obj_id_t::hi)>(fNTupleIndex)},
-      kDistributionKeyDefault, kAttributeKeyFooter, kCidMetadata);
+      kDistributionKeyDefault, kAttributeKeyFooter);
    fNTupleAnchor.fLenFooter = lenFooter;
    fNTupleAnchor.fNBytesFooter = nbytes;
 }
@@ -407,7 +411,7 @@ void ROOT::Experimental::Internal::RPageSinkDaos::WriteNTupleAnchor()
    fNTupleAnchor.Serialize(buffer.get());
    fDaosContainer->WriteSingleAkey(
       buffer.get(), ntplSize, daos_obj_id_t{kOidLowMetadata, static_cast<decltype(daos_obj_id_t::hi)>(fNTupleIndex)},
-      kDistributionKeyDefault, kAttributeKeyAnchor, kCidMetadata);
+      kDistributionKeyDefault, kAttributeKeyAnchor, kCidAnchor);
 }
 
 std::unique_ptr<ROOT::Internal::RPageSink>
@@ -426,8 +430,8 @@ ROOT::Experimental::Internal::RPageSourceDaos::RPageSourceDaos(std::string_view
    EnableDefaultMetrics("RPageSourceDaos");
 
    auto args = ParseDaosURI(uri);
-   auto pool = std::make_shared<RDaosPool>(args.fPoolLabel);
-   fDaosContainer = std::make_unique<RDaosContainer>(pool, args.fContainerLabel);
+   auto pool = std::make_unique<RDaosPool>(args.fPoolLabel);
+   fDaosContainer = std::make_unique<RDaosContainer>(std::move(pool), args.fContainerLabel);
 }
 
 ROOT::Experimental::Internal::RPageSourceDaos::~RPageSourceDaos()
@@ -447,10 +451,7 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadStructureImpl()
    fAnchor = *ntupleLocator.fAnchor;
    fNTupleIndex = ntupleLocator.GetIndex();
 
-   auto oclass = RDaosObject::ObjClassId(fAnchor.fObjClass);
-   if (oclass.IsUnknown())
-      throw ROOT::RException(R__FAIL("LoadStructureImpl: unknown object class " + fAnchor.fObjClass));
-   fDaosContainer->SetDefaultObjectClass(oclass);
+   fDaosContainer->SetDefaultObjectClass(fAnchor.fObjClass);
 
    // Reserve enough space for the compressed and the uncompressed header/footer (see AttachImpl)
    const auto bufSize =
@@ -463,12 +464,12 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadStructureImpl()
    daos_obj_id_t oidMetadata{kOidLowMetadata, static_cast<decltype(daos_obj_id_t::hi)>(fNTupleIndex)};
 
    if ((err = fDaosContainer->ReadSingleAkey(fStructureBuffer.fPtrHeader, fAnchor.fNBytesHeader, oidMetadata,
-                                             kDistributionKeyDefault, kAttributeKeyHeader, kCidMetadata))) {
+                                             kDistributionKeyDefault, kAttributeKeyHeader))) {
       throw ROOT::RException(R__FAIL("LoadStructureImpl: cannot load header: " + std::to_string(err)));
    }
 
    if ((err = fDaosContainer->ReadSingleAkey(fStructureBuffer.fPtrFooter, fAnchor.fNBytesFooter, oidMetadata,
-                                             kDistributionKeyDefault, kAttributeKeyFooter, kCidMetadata))) {
+                                             kDistributionKeyDefault, kAttributeKeyFooter))) {
       throw ROOT::RException(R__FAIL("LoadStructureImpl: cannot load footer: " + std::to_string(err)));
    }
 }
@@ -496,7 +497,7 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadPageListImpl(const RNTup
 {
    daos_obj_id_t oidPageList{kOidLowPageList, static_cast<decltype(daos_obj_id_t::hi)>(fNTupleIndex)};
    fDaosContainer->ReadSingleAkey(buffer, locator.GetNBytesOnStorage(), oidPageList, kDistributionKeyDefault,
-                                  locator.GetPosition<RNTupleLocatorObject64>().GetLocation(), kCidMetadata);
+                                  locator.GetPosition<RNTupleLocatorObject64>().GetLocation());
 }
 
 std::string ROOT::Experimental::Internal::RPageSourceDaos::GetObjectClass() const
@@ -514,8 +515,12 @@ void ROOT::Experimental::Internal::RPageSourceDaos::LoadSealedPageImpl(const RNT
 
 std::unique_ptr<ROOT::Internal::RPageSource> ROOT::Experimental::Internal::RPageSourceDaos::CloneImpl() const
 {
-   auto clone = new RPageSourceDaos(fNTupleName, fURI, fOptions);
-   return std::unique_ptr<RPageSourceDaos>(clone);
+   auto clone = std::make_unique<RPageSourceDaos>(fNTupleName, fURI, fOptions);
+   clone->fAnchor = fAnchor;
+   clone->fNTupleIndex = fNTupleIndex;
+   if (!fAnchor.fObjClass.empty())
+      clone->fDaosContainer->SetDefaultObjectClass(fAnchor.fObjClass);
+   return clone;
 }
 
 std::vector<std::unique_ptr<RCluster>>

tree/ntuple/src/libdaos_mock/libdaos_mock.cxx

@@ -270,6 +270,9 @@ int daos_oclass_id2name(daos_oclass_id_t oc_id, char *name)
    case OC_RP_XSF:
       strcpy(name, "RP_XSF"); // NOLINT
       return 0;
+   case OC_UNKNOWN:
+      strcpy(name, "UNKNOWN"); // NOLINT
+      return 0;
    }
    return -1;
 }

tree/ntuple/test/ntuple_storage_daos.cxx

@@ -81,6 +81,38 @@ TEST_F(RPageStorageDaos, Basics)
    }
 }
 
+TEST_F(RPageStorageDaos, Show)
+{
+   std::string daosUri = RegisterLabel("ntuple-test-show");
+   const std::string_view ntupleName("ntuple");
+   auto model = RNTupleModel::Create();
+   auto wrPt = model->MakeField<float>("pt");
+
+   {
+      auto writer = RNTupleWriter::Recreate(std::move(model), ntupleName, daosUri);
+
+      *wrPt = 42.0;
+      writer->Fill();
+   }
+
+   ROOT::RNTupleReadOptions opts;
+   opts.SetClusterCache(ROOT::RNTupleReadOptions::EClusterCache::kOff);
+   auto reader = RNTupleReader::Open(ntupleName, daosUri);
+   EXPECT_EQ(1U, reader->GetNEntries());
+
+   std::ostringstream os;
+   // Exercises RPageSourceDaos::CloneImpl() because the display reader is cloned
+   reader->Show(0, os);
+   // clang-format off
+   std::string expect{
+R"({
+  "pt": 42
+}
+)" };
+   // clang-format on
+   EXPECT_EQ(expect, os.str());
+}
+
 TEST_F(RPageStorageDaos, Extended)
 {
    std::string daosUri = RegisterLabel("ntuple-test-extended");
@@ -124,38 +156,26 @@ TEST_F(RPageStorageDaos, Extended)
 TEST_F(RPageStorageDaos, Options)
 {
    std::string daosUri = RegisterLabel("ntuple-test-options");
-   const std::string_view ntupleName("ntuple");
-   {
-      auto model = RNTupleModel::Create();
 
-      RNTupleWriteOptionsDaos options;
-      options.SetObjectClass("UNKNOWN");
-      try {
-         auto ntuple = RNTupleWriter::Recreate(std::move(model), ntupleName, daosUri, options);
-         FAIL() << "unknown object class should throw";
-      } catch (const ROOT::RException &err) {
-         EXPECT_THAT(err.what(), testing::HasSubstr("UNKNOWN"));
-      }
-   }
+   for (const auto oclass : {"UNKNOWN", "RP_XSF"}) {
+      {
+         auto model = RNTupleModel::Create();
+         model->MakeField<float>("pt");
 
-   {
-      auto model = RNTupleModel::Create();
-      model->MakeField<float>("pt");
+         RNTupleWriteOptionsDaos options;
+         options.SetObjectClass(oclass);
+         auto writer = RNTupleWriter::Recreate(std::move(model), oclass, daosUri, options);
+         writer->Fill();
+      }
 
-      RNTupleWriteOptionsDaos options;
-      options.SetObjectClass("RP_XSF");
-      auto ntuple = RNTupleWriter::Recreate(std::move(model), ntupleName, daosUri, options);
-      ntuple->Fill();
-      ntuple->CommitCluster();
+      auto readOptions = RNTupleReadOptions();
+      ROOT::Internal::RNTupleReadOptionsManip::SetClusterBunchSize(readOptions, 3);
+      ROOT::Experimental::Internal::RPageSourceDaos source(oclass, daosUri, readOptions);
+      source.Attach();
+      EXPECT_STREQ(oclass, source.GetObjectClass().c_str());
+      EXPECT_EQ(3U, ROOT::Internal::RNTupleReadOptionsManip::GetClusterBunchSize(source.GetReadOptions()));
+      EXPECT_EQ(1U, source.GetNEntries());
    }
-
-   auto readOptions = RNTupleReadOptions();
-   ROOT::Internal::RNTupleReadOptionsManip::SetClusterBunchSize(readOptions, 3);
-   ROOT::Experimental::Internal::RPageSourceDaos source(ntupleName, daosUri, readOptions);
-   source.Attach();
-   EXPECT_STREQ("RP_XSF", source.GetObjectClass().c_str());
-   EXPECT_EQ(3U, ROOT::Internal::RNTupleReadOptionsManip::GetClusterBunchSize(source.GetReadOptions()));
-   EXPECT_EQ(1U, source.GetNEntries());
 }
 
 TEST_F(RPageStorageDaos, LoadSealedPage)