Fix DirRemover should sync before delete mark file

[xiaozongyang]

Describe Your Changes

This PR is trying to fix #649.

I deploy victorialogs in k8s, and mount a OSSFS2 persistent volume as the data directory. The vl pod meets the panic issue described in #649.

And I read the source code of dir_remover.go, and I found that operation sequence is

1. sync dir path
2. remove delete file
3. remove dir

And When I use fuse-based cloud FS, there is a metadata cache. If removing the delete file is completed, the metadata is not newest, it will meet the dir not empty error.

So in this PR, I will change the opeartion sequece to

1. remove delete file
2. sync dir path
3. remove dir

Checklist

The following checks are mandatory:

• My change adheres to VictoriaMetrics contributing guidelines.
• My change adheres to VictoriaMetrics development goals.

[xiaozongyang]

@func25 @valyala @jiekun
Can I build victorialogs docker image locally? So I can test this fix in my test k8s cluster/

[jiekun]

Hello. You can check the Makefile on the root dir of the repo. I believe you need something like this for binary:

make release-victoria-logs

or the following for docker image

make package-victoria-logs

see: https://docs.victoriametrics.com/victorialogs/quickstart/#building-from-source-code

[xiaozongyang]

@jiekun thanks! I've found the Dockerfile in VictoriaLogs/app/victoria-logs/deployment and use it build a tmp image for test.

[xiaozongyang]

@jiekun
I test with my new image, the panic disappeared. Would you mind reviewing this PR?

I'll run this image for 1 day and observe the deletion behavior.

[jiekun]

thanks! I've found the Dockerfile in VictoriaLogs/app/victoria-logs/deployment and use it build a tmp image for test.

That should work. Just in case we have some other necessary things for building the image, I will still recommend using the makefile, and use these super simple commands for compiling binary and docker image, maybe for next time.

[xiaozongyang]

@jiekun Thanks for your suggestions! I'll use Makefile next time.

And should I push a PR against the original repo instead of in the vendor dir?

[func25]

Please create a PR in the VictoriaMetrics repository and wait for it to be merged. After that, you can update the go.mod file to use the new vendor dependency in this PR.

[jiekun]

(Maybe it's worth mentioning it in the contributor guide, regarding how we vendor dependencies and where changes should be made to.)

[xiaozongyang]

hi @func25 @jiekun I open a new PR in repo VictoriaMetrics VictoriaMetrics/VictoriaMetrics#9709, and this PR will be closed.