-
Notifications
You must be signed in to change notification settings - Fork 27
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.
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
Added the ability to specify a list of data/meta folders for the cache #1191
Conversation
config/config.go
Outdated
viper.SetDefault("Cache.MetaLocations", []string{filepath.Join(param.Cache_DataLocation.GetString(), "meta")}) | ||
} else { | ||
viper.SetDefault("Cache.DataLocations", []string{"/run/pelican/xcache/data"}) | ||
viper.SetDefault("Cache.MetaLocations", []string{"/run/pelican/xcache/meta"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the name xcache
is taboo and one of the "swear words." Is it too late to replace it with something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change it, no worries.
#oss.space meta {{.Cache.DataLocation}}/meta* | ||
#oss.space data {{.Cache.DataLocation}}/data* | ||
oss.localroot {{.Cache.LocalRoot}} | ||
pfc.spaces data meta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the comment above explaining that this only works if meta/data
are different devices, I'm worried about someone setting the deprecated Cache.DataLocation
that now enables this previously-disabled feature. Perhaps we can keep the behavior of Cache.DataLocation
the same as it was and not have any defaults for these values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like that above comment isn't correct anymore. I was able to get this to work on the same device. I'm unsure why I put that there originally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked with BrianB about this. We've decided it's probably an erroneous comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Allows a used to specify multiple cache/meta data file locations