Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ntuple] Improve error handling of RNTupleParallelWriter #17368

Merged
merged 3 commits into from
Jan 8, 2025
Merged

[ntuple] Improve error handling of RNTupleParallelWriter #17368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
090d873
[ntuple] Correct exception message in RNTupleParallelWriter
hahnjo Jan 7, 2025
d7ce86e
[ntuple] Remove outdated comment
hahnjo Jan 7, 2025
0d55b49
[ntuple] Forbid RNTupleParallelWriter with a non-binary TFile
hahnjo Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions tree/ntuple/v7/src/RNTupleParallelWriter.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
#include <ROOT/RPageStorage.hxx>
#include <ROOT/RPageStorageFile.hxx>

#include <TDirectory.h>
#include <TError.h>
#include <TFile.h>

namespace {

Expand Down Expand Up @@ -118,7 +120,7 @@ ROOT::Experimental::RNTupleParallelWriter::RNTupleParallelWriter(std::unique_ptr
: fSink(std::move(sink)), fModel(std::move(model)), fMetrics("RNTupleParallelWriter")
{
if (fModel->GetRegisteredSubfields().size() > 0) {
throw RException(R__FAIL("cannot create an RNTupleWriter from a model with registered subfields"));
throw RException(R__FAIL("cannot create an RNTupleParallelWriter from a model with registered subfields"));
}
fModel->Freeze();
fSink->Init(*fModel.get());
Expand Down Expand Up @@ -168,6 +170,16 @@ std::unique_ptr<ROOT::Experimental::RNTupleParallelWriter>
ROOT::Experimental::RNTupleParallelWriter::Append(std::unique_ptr<RNTupleModel> model, std::string_view ntupleName,
TDirectory &fileOrDirectory, const RNTupleWriteOptions &options)
{
auto file = fileOrDirectory.GetFile();
if (!file) {
throw RException(
R__FAIL("RNTupleParallelWriter only supports writing to a ROOT file. Cannot write into a directory "
"that is not backed by a file"));
}
if (!file->IsBinary()) {
throw RException(R__FAIL("RNTupleParallelWriter only supports writing to a ROOT file. Cannot write into " +
std::string(file->GetName())));
}
if (!options.GetUseBufferedWrite()) {
throw RException(R__FAIL("parallel writing requires buffering"));
}
Expand All @@ -182,9 +194,6 @@ std::shared_ptr<ROOT::Experimental::RNTupleFillContext> ROOT::Experimental::RNTu
std::lock_guard g(fMutex);

auto model = fModel->Clone();

// TODO: Think about honoring RNTupleWriteOptions::SetUseBufferedWrite(false); this requires synchronization on every
// call to CommitPage() *and* preparing multiple cluster descriptors in parallel!
auto sink = std::make_unique<Internal::RPageSinkBuf>(std::make_unique<RPageSynchronizingSink>(*fSink, fSinkMutex));

// Cannot use std::make_shared because the constructor of RNTupleFillContext is private. Also it would mean that the
Expand Down
12 changes: 11 additions & 1 deletion tree/ntuple/v7/test/ntuple_parallel_writer.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,20 @@ TEST(RNTupleParallelWriter, ForbidModelWithSubfields)
FAIL() << "should not able to create a writer using a model with registered subfields";
} catch (const ROOT::RException &err) {
EXPECT_THAT(err.what(),
testing::HasSubstr("cannot create an RNTupleWriter from a model with registered subfields"));
testing::HasSubstr("cannot create an RNTupleParallelWriter from a model with registered subfields"));
}
}

TEST(RNTupleParallelWriter, ForbidNonRootTFiles)
{
FileRaii fileGuard("test_ntuple_parallel_forbid_xml.xml");

auto model = RNTupleModel::Create();
auto file = std::unique_ptr<TFile>(TFile::Open(fileGuard.GetPath().c_str(), "RECREATE"));
// Opening an XML TFile should fail
EXPECT_THROW(RNTupleParallelWriter::Append(std::move(model), "ntpl", *file), ROOT::RException);
}

TEST(RNTupleFillContext, FlushColumns)
{
FileRaii fileGuard("test_ntuple_context_flush.root");
Expand Down
Loading