Skip to content
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

"Must-remove" files cannot be removed because "file exists" #6396

Closed
pludov opened this issue Jun 3, 2024 · 4 comments
Closed

"Must-remove" files cannot be removed because "file exists" #6396

pludov opened this issue Jun 3, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@pludov
Copy link
Contributor

pludov commented Jun 3, 2024

Describe the bug

Running VictoriaMetrics on vXFS (vagrant filesystem) via NFS sharing consistently produces a panic error about "must-remove" files that cannot be removed, because "file exists":

metrics.1.jzx9uky49d4m@apl20533app01    | panic: FATAL: cannot remove "/victoria-metrics-data/data/small/2024_06/6_6_20240603100215.600_20240603100215.600_17D5763D06AA420C.must-remove.1717408837293404005": unlinkat /victoria-metrics-data/data/small/2024_06/6_6_20240603100215.600_20240603100215.600_17D5763D06AA420C.must-remove.1717408837293404005: file exists

On our case, this behavior seems specific to the use of vXFS filesystem (vagrant proprietary) over NFS. It cannot be reproduced with ext4 or xfs filesystems. Also tried changing various NFS option with no impact

To Reproduce

Run victoria metrics with data stored on a NFS mount backed by a vXFS filesystem

Version

victoria-metrics-20221007-005009-tags-v1.82.0-0-g832276064

Logs

No response

Screenshots

No response

Used command-line flags

No response

Additional information

Using strace reveals that the unlinkat syscall actually returns errno EEXISTS which is not covered by the current NFS :

11:05:40 newfstatat(AT_FDCWD, "/victoria-metrics-data/indexdb/17D48A952B236F56/462_1_17D48A952C6BADB1.must-remove.1717149731212483987", 0xc0002489f8, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)
11:05:40 renameat(AT_FDCWD, "/victoria-metrics-data/indexdb/17D48A952B236F56/462_1_17D48A952C6BADB1", AT_FDCWD, "/victoria-metrics-data/indexdb/17D48A952B236F56/462_1_17D48A952C6BADB1.must-remove.1717149731212483987" <unfinished ...>
11:05:40 <... renameat resumed>) = 0
11:05:40 unlinkat(AT_FDCWD, "/victoria-metrics-data/indexdb/17D48A952B236F56/462_1_17D48A952C6BADB1.must-remove.1717149731212483987", 0) = -1 EISDIR (Is a directory)
11:05:40 unlinkat(AT_FDCWD, "/victoria-metrics-data/indexdb/17D48A952B236F56/462_1_17D48A952C6BADB1.must-remove.1717149731212483987", AT_REMOVEDIR) = -1 EEXIST (File exists)
11:05:40 openat(AT_FDCWD, "/victoria-metrics-data/indexdb/17D48A952B236F56", O_RDONLY|O_CLOEXEC) = 18
11:05:40 unlinkat(18, "462_1_17D48A952C6BADB1.must-remove.1717149731212483987", 0) = -1 EISDIR (Is a directory)
11:05:40 newfstatat(18, "462_1_17D48A952C6BADB1.must-remove.1717149731212483987", {st_mode=S_IFDIR|0755, st_size=1024, ...}, AT_SYMLINK_NOFOLLOW) = 0
11:05:40 openat(18, "462_1_17D48A952C6BADB1.must-remove.1717149731212483987", O_RDONLY|O_CLOEXEC) = 22
11:05:40 getdents64(22, /* 7 entries */, 8192) = 224
11:05:40 getdents64(22, /* 0 entries */, 8192) = 0
11:05:40 unlinkat(22, "metaindex.bin", 0) = 0
11:05:40 unlinkat(22, "index.bin", 0) = 0
11:05:40 unlinkat(22, "items.bin", 0) = 0
11:05:40 unlinkat(22, "lens.bin", 0 <unfinished ...>
11:05:40 <... unlinkat resumed>) = 0
11:05:40 unlinkat(22, "metadata.json", 0) = 0
11:05:40 close(22)          = 0
11:05:40 unlinkat(18, "462_1_17D48A952C6BADB1.must-remove.1717149731212483987", AT_REMOVEDIR) = -1 EEXIST (File exists)
11:05:40 close(18)          = 0
11:05:40 write(2, "2024-05-31T10:05:40.433Z\tpanic\tVictoriaMetrics/lib/fs/dir_remover.go:53\tFATAL: cannot remove \"/victoria-metrics-data/indexdb/17D48A952B236F56/462_1_17D48A952C6BADB1.must-remove.1717149731212483987\": unlinkat /victoria-metrics-data/indexdb/17D48A952B236F56/462_1_17D48A952C6BADB1.must-remove.1717149731212483987: file exists\n", 324) = 324

We are using NFS, so the unlink_at failure is expected (expected NFS race...), but it is not catched by VM due to the code used here (EEXIST) vs the one searched by VM (ENOTEMPTY) in isTemporaryNFSError (dir_remover.go).

@Haleygo
Copy link
Contributor

Haleygo commented Jun 4, 2024

related #162

hagen1778 pushed a commit to pludov/VictoriaMetrics that referenced this issue Jun 4, 2024
hagen1778 added a commit that referenced this issue Jun 4, 2024
…TEMPTY (#6398)

### Describe Your Changes

Fix for issue #6396: according to rmdir manpage, ENOTEMPTY and EEXIST
should be treated equally

#6396

### Checklist

The following checks are **mandatory**:

- [x ] My change adheres [VictoriaMetrics contributing
guidelines](https://docs.victoriametrics.com/contributing/).

---------

Co-authored-by: Ludovic Pollet <ludovic.pollet@exfo.com>
Co-authored-by: hagen1778 <roman@victoriametrics.com>
hagen1778 pushed a commit that referenced this issue Jun 4, 2024
…TEMPTY (#6398)

### Describe Your Changes

Fix for issue #6396: according to rmdir manpage, ENOTEMPTY and EEXIST
should be treated equally

#6396

### Checklist

The following checks are **mandatory**:

- [x ] My change adheres [VictoriaMetrics contributing
guidelines](https://docs.victoriametrics.com/contributing/).

---------

Co-authored-by: Ludovic Pollet <ludovic.pollet@exfo.com>
Co-authored-by: hagen1778 <roman@victoriametrics.com>
(cherry picked from commit 3ddae77)
hagen1778 pushed a commit that referenced this issue Jun 4, 2024
…TEMPTY (#6398)

### Describe Your Changes

Fix for issue #6396: according to rmdir manpage, ENOTEMPTY and EEXIST
should be treated equally

#6396

### Checklist

The following checks are **mandatory**:

- [x ] My change adheres [VictoriaMetrics contributing
guidelines](https://docs.victoriametrics.com/contributing/).

---------

Co-authored-by: Ludovic Pollet <ludovic.pollet@exfo.com>
Co-authored-by: hagen1778 <roman@victoriametrics.com>
(cherry picked from commit 3ddae77)
hagen1778 pushed a commit that referenced this issue Jun 4, 2024
…TEMPTY (#6398)

### Describe Your Changes

Fix for issue #6396: according to rmdir manpage, ENOTEMPTY and EEXIST
should be treated equally

#6396

### Checklist

The following checks are **mandatory**:

- [x ] My change adheres [VictoriaMetrics contributing
guidelines](https://docs.victoriametrics.com/contributing/).

---------

Co-authored-by: Ludovic Pollet <ludovic.pollet@exfo.com>
Co-authored-by: hagen1778 <roman@victoriametrics.com>
(cherry picked from commit 3ddae77)
@hagen1778 hagen1778 self-assigned this Jun 4, 2024
@hagen1778
Copy link
Collaborator

Thanks for #6398!
It was merged and will be available in the next release. Meanwhile, VictoriaMetrics can be built from sources.

@valyala
Copy link
Collaborator

valyala commented Jun 7, 2024

The bugfix for this issue has been included in v1.97.5 LTS release and in v1.102.0-rc1 release.

@valyala valyala closed this as completed Jun 7, 2024
@valyala
Copy link
Collaborator

valyala commented Jun 7, 2024

FYI, the bugfix has been included also into v1.93.15 LTS release.

rtm0 pushed a commit to rtm0/VictoriaMetrics that referenced this issue Jun 8, 2024
…TEMPTY (VictoriaMetrics#6398)

### Describe Your Changes

Fix for issue VictoriaMetrics#6396: according to rmdir manpage, ENOTEMPTY and EEXIST
should be treated equally

VictoriaMetrics#6396

### Checklist

The following checks are **mandatory**:

- [x ] My change adheres [VictoriaMetrics contributing
guidelines](https://docs.victoriametrics.com/contributing/).

---------

Co-authored-by: Ludovic Pollet <ludovic.pollet@exfo.com>
Co-authored-by: hagen1778 <roman@victoriametrics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants