diff --git a/tree/ntuple/inc/ROOT/RDaos.hxx b/tree/ntuple/inc/ROOT/RDaos.hxx index 0ef258392122f..1da1b03e55ba4 100644 --- a/tree/ntuple/inc/ROOT/RDaos.hxx +++ b/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 fPool; - ObjClassId_t fDefaultObjectClass{OC_SX}; + std::unique_ptr 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 pool, std::string_view containerId, bool create = false); + RDaosContainer(std::unique_ptr pool, std::string_view containerId, bool create = false); ~RDaosContainer(); ObjClassId_t GetDefaultObjectClass() const { return fDefaultObjectClass; } diff --git a/tree/ntuple/inc/ROOT/RNTupleWriteOptionsDaos.hxx b/tree/ntuple/inc/ROOT/RNTupleWriteOptionsDaos.hxx index 9bf245e4f458a..b296079f18adb 100644 --- a/tree/ntuple/inc/ROOT/RNTupleWriteOptionsDaos.hxx +++ b/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; diff --git a/tree/ntuple/inc/ROOT/RPageStorage.hxx b/tree/ntuple/inc/ROOT/RPageStorage.hxx index 46447bea2b0a3..153c21a25063c 100644 --- a/tree/ntuple/inc/ROOT/RPageStorage.hxx +++ b/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 CloneImpl() const = 0; // Only called if a task scheduler is set. No-op be default. virtual void UnzipClusterImpl(ROOT::Internal::RCluster *cluster); diff --git a/tree/ntuple/src/RDaos.cxx b/tree/ntuple/src/RDaos.cxx index 4a7ac4ba2d50c..a3ec4da3ed797 100644 --- a/tree/ntuple/src/RDaos.cxx +++ b/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 pool, +ROOT::Experimental::Internal::RDaosContainer::RDaosContainer(std::unique_ptr pool, std::string_view containerId, bool create) - : fPool(pool) + : fPool(std::move(pool)) { daos_cont_info_t containerInfo{}; diff --git a/tree/ntuple/src/RPageStorageDaos.cxx b/tree/ntuple/src/RPageStorageDaos.cxx index 7a866cf40bfa8..cc4ea54ebd051 100644 --- a/tree/ntuple/src/RPageStorageDaos.cxx +++ b/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(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(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(args.fPoolLabel); + auto pool = std::make_unique(args.fPoolLabel); - fDaosContainer = std::make_unique(pool, args.fContainerLabel, /*create =*/true); - fDaosContainer->SetDefaultObjectClass(oclass); + fDaosContainer = std::make_unique(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(fNTupleIndex)}, kDistributionKeyDefault, - offsetData, kCidMetadata); + bufPageListZip.get(), + szPageListZip, + daos_obj_id_t{kOidLowPageList, static_cast(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(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(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(fNTupleIndex)}, - kDistributionKeyDefault, kAttributeKeyAnchor, kCidMetadata); + kDistributionKeyDefault, kAttributeKeyAnchor, kCidAnchor); } std::unique_ptr @@ -426,8 +430,8 @@ ROOT::Experimental::Internal::RPageSourceDaos::RPageSourceDaos(std::string_view EnableDefaultMetrics("RPageSourceDaos"); auto args = ParseDaosURI(uri); - auto pool = std::make_shared(args.fPoolLabel); - fDaosContainer = std::make_unique(pool, args.fContainerLabel); + auto pool = std::make_unique(args.fPoolLabel); + fDaosContainer = std::make_unique(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(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(fNTupleIndex)}; fDaosContainer->ReadSingleAkey(buffer, locator.GetNBytesOnStorage(), oidPageList, kDistributionKeyDefault, - locator.GetPosition().GetLocation(), kCidMetadata); + locator.GetPosition().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::Experimental::Internal::RPageSourceDaos::CloneImpl() const { - auto clone = new RPageSourceDaos(fNTupleName, fURI, fOptions); - return std::unique_ptr(clone); + auto clone = std::make_unique(fNTupleName, fURI, fOptions); + clone->fAnchor = fAnchor; + clone->fNTupleIndex = fNTupleIndex; + if (!fAnchor.fObjClass.empty()) + clone->fDaosContainer->SetDefaultObjectClass(fAnchor.fObjClass); + return clone; } std::vector> diff --git a/tree/ntuple/src/libdaos_mock/libdaos_mock.cxx b/tree/ntuple/src/libdaos_mock/libdaos_mock.cxx index 20d4ce5cf196b..f0561731329cc 100644 --- a/tree/ntuple/src/libdaos_mock/libdaos_mock.cxx +++ b/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; } diff --git a/tree/ntuple/test/ntuple_storage_daos.cxx b/tree/ntuple/test/ntuple_storage_daos.cxx index 1ee84e5371c17..fb385ab694b37 100644 --- a/tree/ntuple/test/ntuple_storage_daos.cxx +++ b/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("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("pt"); - { - auto model = RNTupleModel::Create(); - model->MakeField("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)