Skip to content

Commit 740b329

Browse files
committed
Detect failed reads when a tree is missing from a DF
1 parent 27f5bda commit 740b329

1 file changed

Lines changed: 51 additions & 1 deletion

File tree

‎Framework/AnalysisSupport/src/DataInputDirector.cxx‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,14 @@
3030
#include "TObjString.h"
3131
#include "TMap.h"
3232
#include "TFile.h"
33+
#include "TError.h"
3334

3435
#include <arrow/dataset/file_base.h>
3536
#include <arrow/dataset/dataset.h>
3637
#include <uv.h>
3738
#include <exception>
3839
#include <memory>
40+
#include <string_view>
3941

4042
#if __has_include(<TJAlienFile.h>)
4143
#include <TJAlienFile.h>
@@ -58,6 +60,42 @@ FileNameHolder makeFileNameHolder(std::string fileName)
5860
return holder;
5961
}
6062

63+
namespace
64+
{
65+
// ROOT reports a failed key read (a folder's key list or the object itself) and carries on,
66+
// so the lookup returns nullptr, or an object built from whatever was in the buffer, and a
67+
// failed read looks like a missing object. Count the reports while the watch is alive.
68+
struct RootReadErrorWatch {
69+
RootReadErrorWatch()
70+
: mPrevious(SetErrorHandler(handler))
71+
{
72+
sPrevious = mPrevious;
73+
sErrors = 0;
74+
}
75+
~RootReadErrorWatch()
76+
{
77+
SetErrorHandler(mPrevious);
78+
}
79+
RootReadErrorWatch(RootReadErrorWatch const&) = delete;
80+
RootReadErrorWatch& operator=(RootReadErrorWatch const&) = delete;
81+
82+
[[nodiscard]] bool failed() const { return sErrors > 0; }
83+
84+
static void handler(int level, Bool_t abort, char const* location, char const* msg)
85+
{
86+
if (level >= kError && location && std::string_view{location} == "TKey::ReadFile") {
87+
++sErrors;
88+
}
89+
(sPrevious ? sPrevious : DefaultErrorHandler)(level, abort, location, msg);
90+
}
91+
92+
ErrorHandlerFunc_t mPrevious;
93+
// Errors from other threads are passed on but not counted.
94+
static inline thread_local int sErrors = 0;
95+
static inline ErrorHandlerFunc_t sPrevious = nullptr;
96+
};
97+
} // namespace
98+
6199
DataInputDescriptor::DataInputDescriptor(bool alienSupport, int level, DataInputDirectorContext& context)
62100
: mAlienSupport(alienSupport),
63101
mContext(context),
@@ -605,7 +643,19 @@ try {
605643

606644
for (auto& capability : mFactory.capabilities) {
607645
auto objectPath = capability.lfn2objectPath(fullpath.path());
608-
void* handle = capability.getHandle(rootFS, objectPath);
646+
void* handle = nullptr;
647+
bool readFailed = false;
648+
try {
649+
RootReadErrorWatch watch;
650+
handle = capability.getHandle(rootFS, objectPath);
651+
readFailed = watch.failed();
652+
} catch (...) {
653+
// Sizes from a partial read can be garbage, e.g. giving std::bad_alloc.
654+
std::throw_with_nested(InvalidAODReadError(fmt::format(R"(Unable to look up "{}" in "{}")", objectPath, rootFS->GetFile()->GetName())));
655+
}
656+
if (readFailed) {
657+
throw InvalidAODReadError(fmt::format(R"(Unable to read "{}" from "{}")", objectPath, rootFS->GetFile()->GetName()));
658+
}
609659
if (handle) {
610660
format = capability.factory().format();
611661
creator = capability.factory().deferredOutputStreamer;

0 commit comments

Comments
 (0)