Skip to content

Commit

Permalink
Add a private flag to throw clobbered error messages (#2710)
Browse files Browse the repository at this point in the history
* Add a private flag to throw clobbered error messages in case of write failures.

* added config validation test

* Updated flag name and usage
  • Loading branch information
vipnydav authored Nov 26, 2024
1 parent 537ca67 commit c015e94
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 3 deletions.
14 changes: 13 additions & 1 deletion cfg/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ type FileSystemConfig struct {

KernelListCacheTtlSecs int64 `yaml:"kernel-list-cache-ttl-secs"`

PreconditionErrors bool `yaml:"precondition-errors"`

RenameDirLimit int64 `yaml:"rename-dir-limit"`

TempDir ResolvedPath `yaml:"temp-dir"`
Expand Down Expand Up @@ -459,6 +461,12 @@ func BuildFlagSet(flagSet *pflag.FlagSet) error {

flagSet.StringP("only-dir", "", "", "Mount only a specific directory within the bucket. See docs/mounting for more information")

flagSet.BoolP("precondition-errors", "", true, "Throw Stale NFS file handle error in case the object being synced or read from is modified by some other concurrent process. This helps prevent silent data loss or data corruption.")

if err := flagSet.MarkHidden("precondition-errors"); err != nil {
return err
}

flagSet.IntP("prometheus-port", "", 0, "Expose Prometheus metrics endpoint on this port and a path of /metrics.")

if err := flagSet.MarkHidden("prometheus-port"); err != nil {
Expand Down Expand Up @@ -523,7 +531,7 @@ func BuildFlagSet(flagSet *pflag.FlagSet) error {
return err
}

flagSet.StringP("temp-dir", "", "", "Path to the temporary directory where writes are staged prior to upload to Cloud Storage. (default: system default, likely /tmp)\"")
flagSet.StringP("temp-dir", "", "", "Path to the temporary directory where writes are staged prior to upload to Cloud Storage. (default: system default, likely /tmp)")

flagSet.StringP("token-url", "", "", "A url for getting an access token when the key-file is absent.")

Expand Down Expand Up @@ -796,6 +804,10 @@ func BindFlags(v *viper.Viper, flagSet *pflag.FlagSet) error {
return err
}

if err := v.BindPFlag("file-system.precondition-errors", flagSet.Lookup("precondition-errors")); err != nil {
return err
}

if err := v.BindPFlag("metrics.prometheus-port", flagSet.Lookup("prometheus-port")); err != nil {
return err
}
Expand Down
12 changes: 11 additions & 1 deletion cfg/params.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,16 @@
will throw error.
default: "0"

- config-path: "file-system.precondition-errors"
flag-name: "precondition-errors"
type: "bool"
usage: >-
Throw Stale NFS file handle error in case the object being synced or read
from is modified by some other concurrent process. This helps prevent
silent data loss or data corruption.
hide-flag: true
default: true

- config-path: "file-system.rename-dir-limit"
flag-name: "rename-dir-limit"
type: "int"
Expand All @@ -205,7 +215,7 @@
type: "resolvedPath"
usage: >-
Path to the temporary directory where writes are staged prior to upload to
Cloud Storage. (default: system default, likely /tmp)"
Cloud Storage. (default: system default, likely /tmp)
default: ""

- config-path: "file-system.uid"
Expand Down
3 changes: 3 additions & 0 deletions cmd/config_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,7 @@ func TestValidateConfigFile_FileSystemConfigSuccessful(t *testing.T) {
KernelListCacheTtlSecs: 0,
RenameDirLimit: 0,
TempDir: "",
PreconditionErrors: true,
Uid: -1,
HandleSigterm: true,
},
Expand All @@ -533,6 +534,7 @@ func TestValidateConfigFile_FileSystemConfigSuccessful(t *testing.T) {
KernelListCacheTtlSecs: 0,
RenameDirLimit: 0,
TempDir: "",
PreconditionErrors: true,
Uid: -1,
HandleSigterm: true,
},
Expand All @@ -552,6 +554,7 @@ func TestValidateConfigFile_FileSystemConfigSuccessful(t *testing.T) {
KernelListCacheTtlSecs: 300,
RenameDirLimit: 10,
TempDir: cfg.ResolvedPath(path.Join(hd, "temp")),
PreconditionErrors: false,
Uid: 8,
HandleSigterm: true,
},
Expand Down
5 changes: 4 additions & 1 deletion cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ func TestArgsParsing_FileSystemFlags(t *testing.T) {
}{
{
name: "normal",
args: []string{"gcsfuse", "--dir-mode=0777", "--disable-parallel-dirops", "--file-mode=0666", "--o", "ro", "--gid=7", "--ignore-interrupts=false", "--kernel-list-cache-ttl-secs=300", "--rename-dir-limit=10", "--temp-dir=~/temp", "--uid=8", "abc", "pqr"},
args: []string{"gcsfuse", "--dir-mode=0777", "--disable-parallel-dirops", "--file-mode=0666", "--o", "ro", "--gid=7", "--ignore-interrupts=false", "--kernel-list-cache-ttl-secs=300", "--rename-dir-limit=10", "--temp-dir=~/temp", "--uid=8", "--precondition-errors=false", "abc", "pqr"},
expectedConfig: &cfg.Config{
FileSystem: cfg.FileSystemConfig{
DirMode: 0777,
Expand All @@ -653,6 +653,7 @@ func TestArgsParsing_FileSystemFlags(t *testing.T) {
KernelListCacheTtlSecs: 300,
RenameDirLimit: 10,
TempDir: cfg.ResolvedPath(path.Join(hd, "temp")),
PreconditionErrors: false,
Uid: 8,
HandleSigterm: true,
},
Expand All @@ -672,6 +673,7 @@ func TestArgsParsing_FileSystemFlags(t *testing.T) {
KernelListCacheTtlSecs: 0,
RenameDirLimit: 0,
TempDir: "",
PreconditionErrors: true,
Uid: -1,
HandleSigterm: true,
},
Expand All @@ -691,6 +693,7 @@ func TestArgsParsing_FileSystemFlags(t *testing.T) {
KernelListCacheTtlSecs: 0,
RenameDirLimit: 0,
TempDir: "",
PreconditionErrors: true,
Uid: -1,
HandleSigterm: true,
},
Expand Down
1 change: 1 addition & 0 deletions cmd/testdata/valid_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ file-system:
kernel-list-cache-ttl-secs: 300
rename-dir-limit: 10
temp-dir: ~/temp
precondition-errors: false
list:
enable-empty-managed-folders: true
enable-hns: false
Expand Down

0 comments on commit c015e94

Please sign in to comment.