[XrdCmsRedirLocal] Add localroot option for plug-in. Improve fault tolerance of RedirLocal plug-in #1523
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n oss.localroot config in redirector, which caused problems if path of oss.localroot was not available to the redirector and therefore required a dummy path
… available after redirect to local, instead using regular dataserver
abh3
requested changes
Oct 26, 2021
src/XrdCms/XrdCmsRedirLocal.cc
Outdated
| // search for localroot, | ||
| // which manually sets localroot to prepend | ||
| else if (strcmp(word, "xrdcmsredirlocal.localroot") == 0){ | ||
| localroot = std::string(Config.GetWord(true));//to lower case |
There was a problem hiding this comment.
Are you sure you want to restrict localroot to be nothing but lower case? Also, there should be a check that it starts with a slash since root paths should be absolute to avoid unpleasant surprises.
There was a problem hiding this comment.
Good point, fixed. The error checks are in as well
src/XrdCms/XrdCmsRedirLocal.cc
Outdated
| // the redirection loop | ||
| int param = 0; // need it to get Env | ||
| std::string envInfo(EnvInfo->Env(param)); // get already tried hosts | ||
| std::string searchPattern = "&tried=localhost"; //search for this pattern |
There was a problem hiding this comment.
Note that if "tried=" is the first token after the question mark it will not (typically) have a leading '&'. This doesn't take care of that.
There was a problem hiding this comment.
Thanks, corrected. But we're now searching for "tried=localhost" in the full envInfo. Otherwise, it might be missed, if other params are passed first.
…nfig option xrdcmsredirlocal.localroot overwrites older. Look for tried=localhost in full EnvInfo to prevent missing it due to other params. Allow upper- and lower case for localroot. Error checking when localroot doesn't start with /. Refactor reading of config options for plug-in.
|
I am a little behind but it is on my list!
…On Wed, 3 Nov 2021, pkramp wrote:
@pkramp requested your review on: xrootd/xrootd#1523 [XrdCmsRedirLocal] Add localroot option for plug-in. Improve fault tolerance of RedirLocal plug-in.
--
You are receiving this because your review was requested.
Reply to this email directly or view it on GitHub:
#1523 (comment)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Until now, the plug-in used oss.localroot set in the redirector, to calculate the new paths. This led to conflicts described in issue #1278 . Adding a config option with a localroot onl for the plug-in, this issue is resolved.
The second commit increases the fault tolerance: Before this commit, if a client is redirected to the local filesystem, but does not find the file there, a redirection loop is started.
Example:
Open has returned with status [FATAL] Redirect limit has been reached Error while opening at 192.168.122.200:1094: [FATAL] Redirect limit has been reached Redirect trace-back: 0. Redirected from: root://192.168.122.200:1094//autoTest1 to: file://localhost/xrdmnt//autoTest1 1. Retrying: root://192.168.122.200:1094//autoTest1 2. Redirected from: root://192.168.122.200:1094//autoTest1 to: file://localhost/xrdmnt//xrdmnt/autoTest1 3. Retrying: root://192.168.122.200:1094//autoTest1 4. Redirected from: root://192.168.122.200:1094//autoTest1 to: file://localhost/xrdmnt//xrdmnt/xrdmnt/autoTest1 ...With the new changes, the plug-in checks if localroot is in the path AND if the EnvInfo shows that the client has already tried localhost. The result is this:
Redirect trace-back: 0. Redirected from: root://192.168.122.200:1094//autoTest1 to: file://localhost/xrdmnt//autoTest1 1. Retrying: root://192.168.122.200:1094//autoTest1 2. Waited at server request. Resending: root://192.168.122.200:1094//autoTest1 3. Waited at server request. Resending: root://192.168.122.200:1094//autoTest1 4. Redirected from: root://192.168.122.200:1094//autoTest1 to: root://192.168.122.202:1094//autoTest1The client tries a second time and gets redirected to the dataserver it would normally have contacted