Redirect TFile::Open() calls to XRootD protocol if possible #11644
Conversation
|
Starting build on |
amadio
changed the title
amadio
changed the title
|
Build failed on mac1015/cxx17. Errors:
|
|
Build failed on mac11/cxx14. Errors:
|
Guilherme tells me that it's actually TChain that strips the |
amadio
force-pushed
the
redirect-xrootd
branch
from
fba4f3f to
590ebb0
Compare
|
Starting build on |
amadio
force-pushed
the
redirect-xrootd
branch
from
590ebb0 to
78c401f
Compare
|
Starting build on |
|
Please clang-format as well... |
The first call to by this time, the |
|
The Line 485 in 8cc176c (this is in It looks like special-casing Line 2159 in 8cc176c |
|
Removing the special casing leads to this: So that breaks the redirection, since now |
amadio
force-pushed
the
redirect-xrootd
branch
from
78c401f to
9d76be6
Compare
|
Starting build on |
amadio
force-pushed
the
redirect-xrootd
branch
from
9d76be6 to
3b8a227
Compare
|
Starting build on |
|
Build failed on ROOT-ubuntu2004/python3. Errors:
|
|
Build failed on ROOT-debian10-i386/soversion. Errors:
|
amadio
force-pushed
the
redirect-xrootd
branch
from
3b8a227 to
ee9df4a
Compare
|
Starting build on |
|
Starting build on |
|
@Axel-Naumann I've updated the patch to use @pcanal I tried moving the code after line 4057 and using So I moved it back where it was before and use only the input |
Can you give a concrete illustration? |
amadio
force-pushed
the
redirect-xrootd
branch
from
fa67e43 to
fdbafcf
Compare
|
Starting build on |
|
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
Here is a session with $ strace ./dimuon Run2012BC_DoubleMuParked_Muons.root 2>&1 | grep Muons
execve("./dimuon", ["./dimuon", "Run2012BC_DoubleMuParked_Muons.r"...], 0x7ffd2e39de38 /* 49 vars */) = 0
access("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", F_OK) = 0
access("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", R_OK) = 0
openat(AT_FDCWD, "/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", O_RDONLY) = 4
lstat("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
lstat("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
access("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", F_OK) = 0
access("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", R_OK) = 0
openat(AT_FDCWD, "/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", O_RDONLY) = 5
lstat("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
lstat("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
access("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", F_OK) = 0
access("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", R_OK) = 0
openat(AT_FDCWD, "/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", O_RDONLY) = 5
lstat("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
lstat("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
write(1, "Muons with opposite charge: pass"..., 97Muons with opposite charge: pass=24067843 all=31104343 -- eff=77.38 % cumulative eff=39.11 %one with the redirection enabled: $ strace ./dimuon Run2012BC_DoubleMuParked_Muons.root 2>&1 | grep Muons
execve("./dimuon", ["./dimuon", "Run2012BC_DoubleMuParked_Muons.r"...], 0x7ffe7c400bd8 /* 49 vars */) = 0
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", NULL, 0) = 79
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", "root://eoshome-a.cern.ch//eos/us"..., 79) = 79
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", NULL, 0) = 79
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", "root://eoshome-a.cern.ch//eos/us"..., 79) = 79
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", NULL, 0) = 79
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", "root://eoshome-a.cern.ch//eos/us"..., 79) = 79
write(1, "Muons with opposite charge: pass"..., 97Muons with opposite charge: pass=24067843 all=31104343 -- eff=77.38 % cumulative eff=39.11 %and finally, one with the code moved to the place you suggested (using $ strace ./dimuon Run2012BC_DoubleMuParked_Muons.root 2>&1 | grep Muons
execve("./dimuon", ["./dimuon", "Run2012BC_DoubleMuParked_Muons.r"...], 0x7ffccf746158 /* 49 vars */) = 0
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", NULL, 0) = 25
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", "root://eoshome-a.cern.ch/", 25) = 25
access("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", F_OK) = 0
access("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", R_OK) = 0
openat(AT_FDCWD, "/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", O_RDONLY) = 12
lstat("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
lstat("/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", NULL, 0) = 79
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", "root://eoshome-a.cern.ch//eos/us"..., 79) = 79
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", NULL, 0) = 79
getxattr("Run2012BC_DoubleMuParked_Muons.root", "eos.url.xroot", "root://eoshome-a.cern.ch//eos/us"..., 79) = 79
write(1, "Muons with opposite charge: pass"..., 97Muons with opposite charge: pass=24067843 all=31104343 -- eff=77.38 % cumulative eff=39.11 %It is a bit puzzling (doesn't always happen), but as you can see, in the case using |
|
More puzzling, here are two consecutive runs without changing anything (using lcgapp-centos8-physical amadio $ cat test.C
#include "TFile.h"
void test(const char* filename, const char* options)
{
auto f = TFile::Open(filename, options);
}
int main(int argc, char **argv)
{
test(argv[1], argv[2]);
}
$ cat ~/.rootrc && strace ./test test.root 2>&1 | grep "test.root"
TFile.CrossProtocolRedirects: yes
execve("./test", ["./test", "test.root"], 0x7fffd10a02b8 /* 49 vars */) = 0
getxattr("test.root", "eos.url.xroot", NULL, 0) = 54
getxattr("test.root", "eos.url.xroot", "root://eoshome-a.cern.ch//eos/us"..., 54) = 54
access("/eos/home-a/amadio/test.root", F_OK) = 0
access("/eos/home-a/amadio/test.root", R_OK) = 0
openat(AT_FDCWD, "/eos/home-a/amadio/test.root", O_RDONLY) = 11
lstat("/eos/home-a/amadio/test.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
lstat("/eos/home-a/amadio/test.root", {st_mode=S_IFREG|0644, st_size=2244449133, ...}) = 0
$ cat ~/.rootrc && strace ./test test.root 2>&1 | grep "test.root"
TFile.CrossProtocolRedirects: yes
execve("./test", ["./test", "test.root"], 0x7fff277b6488 /* 49 vars */) = 0
getxattr("test.root", "eos.url.xroot", NULL, 0) = 53
getxattr("test.root", "eos.url.xroot", "root://eoshome-a.cern.ch//eos/us"..., 53) = 53So better keep the code where it is, as I never observe this sort of thing in that case. |
|
For the record, the build will fail because some tests based on reference files now have some unexpected warning: These are not caused by my changes, so I will merge this if those are the only failures. |
|
Yup sorry I'm on it. |
This actually quite concerning ... In the use case you show, there should be not practical difference as the filename should be strictly the same before and after the expand. (As opposed to load "/eos/home-a/amadio/Run2012BC_DoubleMuParked_Muons.root" vs "$MY_EOS_DIR/Run2012BC_DoubleMuParked_Muons.root" where the redirection would work only for the 1st one without the expansion). In the first strace, I see: vs which "seems" to mean that goven the exact same input
That seems "consistent" with the "inconsistent" behavior I just talked about. So there is something wrong about (a) it is semantically necessary to call it so since we have a running failing example of the odd behavior we may as well investigate it now. |
Good question, and I'm not sure. This seems to be a problem with EOS (see below). @apeters1971 Do you know what could cause this? To reproduce, all you need is to copy a file with a different name, then call $ cp hsimple.root mytest.root
$ getfattr -n eos.url.xroot hsimple.root && getfattr -n eos.url.xroot mytest.root
# file: hsimple.root
eos.url.xroot="root://eoshome-a.cern.ch//eos/user/a/amadio/hsimple.root"
# file: mytest.root
eos.url.xroot="root://eoshome-a.cern.ch/"So I guess we could move the code after the expansion after all, as the problem is not coming from ROOT, but from EOS. When the wrong URL is returned, all that happens is that the opening via XRootD fails and it falls back to use FUSE, so I guess it should still be fine to merge this, if only for allowing people to test more easily once ROOT master is installed somewhere in CVMFS. |
amadio
force-pushed
the
redirect-xrootd
branch
from
fdbafcf to
bdac0fc
Compare
|
Starting build on |
amadio
force-pushed
the
redirect-xrootd
branch
from
bdac0fc to
f4c67e5
Compare
|
Starting build on |
|
Build failed on mac12/noimt. Warnings:
|
amadio
dismissed
pcanal’s stale review
I've moved the code to the suggested place as the error I saw is something in EOS (can be reproduced without ROOT being involved). I'd like to merge to let others test more widely before the release by using an installation of ROOT master from CVMFS.
|
Ah! Perfect timing, Philippe! Thanks! Merging. |
In a meeting in IT I saw that ~200PB/year are being read from the
eoshomeinstance via fuse, and I thought that maybe ROOT at least could try to do something smart and just redirect fuse traffic to XRootD if possible. The patch in this pull request does just that. When I talked to Axel about making this work, we wanted the redirection to not happen when usingfile://file.root, but it created some inconsistencies when usingTFile::Openvs usingTChainso the final implementation always attempts to redirect.Below is a demo session of ROOT opening a file on EOS with (a former version of) the patch:
and with a slightly modified
df102_NanoAODDimuonAnalysistutorial to avoidTChain:void df102_NanoAODDimuonAnalysis(const char* filename) { // Enable multi-threading ROOT::EnableImplicitMT(); - // std::cout << "Using filename: " << filename << std::endl; + auto f = TFile::Open(filename); + auto t = f->Get<TTree>("Events"); - ROOT::RDataFrame df("Events", filename); + ROOT::RDataFrame df(*t);I got the following:
Note: With the final version of the patch, instead of using
file://, which now does not disable the redirection anymore, one should addTFile.CrossProtocolRedirects: noto theirrootrcconfiguration file in order to disable this feature (I checked that it works).