From c79bf3925c39e79c9db58b47c96ca0edc30ef6f7 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Sat, 30 Mar 2024 07:29:24 +0200 Subject: [PATCH] Revert "app/vmselect: make vmselect resilient to absence of cache folder (#5987)" This reverts commit cb23685681d2dc42499c3aa485679964e79f70b7. Reason for revert: the "fix" may hide programming bugs related to incorrect creation of folders before their use. This may complicate detecting and fixing such bugs in the future. There are the following fixes for the issue https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5985 : - To configure the OS to do not drop data from the system-wide temporary directory (aka /tmp). - To run VictoriaMetrics with -cacheDataPath command-line flag, which points to the directory, which cannot be removed automatically by the OS. The case when the user accidentally deletes the directory with some files created by VictoriaMetrics shouldn't be considered as expected, so VictoriaMetrics shouldn't try resolving this case automatically. It is much better from operation and debuggability PoV is to crash with the clear `directory doesn't exist` error in this case. --- app/vmselect/netstorage/tmp_blocks_file.go | 18 +----------------- docs/CHANGELOG.md | 1 - lib/fs/fs.go | 7 ------- 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/app/vmselect/netstorage/tmp_blocks_file.go b/app/vmselect/netstorage/tmp_blocks_file.go index 7e20d75fb..78288d538 100644 --- a/app/vmselect/netstorage/tmp_blocks_file.go +++ b/app/vmselect/netstorage/tmp_blocks_file.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "sync" "github.com/VictoriaMetrics/VictoriaMetrics/lib/bytesutil" @@ -108,7 +107,7 @@ func (tbf *tmpBlocksFile) WriteBlockRefData(b []byte) (tmpBlockAddr, error) { // Slow path: flush the data from tbf.buf to file. if tbf.f == nil { - f, err := createTemp(tmpBlocksDir) + f, err := os.CreateTemp(tmpBlocksDir, "") if err != nil { return addr, err } @@ -123,21 +122,6 @@ func (tbf *tmpBlocksFile) WriteBlockRefData(b []byte) (tmpBlockAddr, error) { return addr, nil } -// createTemp creates new temporary file in the path dir. -// If path doesn't exist, it will try creating it. -func createTemp(path string) (*os.File, error) { - f, err := os.CreateTemp(path, "") - if err == nil { - return f, nil - } - if os.IsNotExist(err) || strings.Contains(err.Error(), "no such file or directory") { - // try re-creating the path and trying again - fs.MustMkdirIfNotExist(path) - return os.CreateTemp(path, "") - } - return nil, err -} - // Len() returnt tbf size in bytes. func (tbf *tmpBlocksFile) Len() uint64 { return tbf.offset diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 80c1ca9d6..70af834c9 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -68,7 +68,6 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). * BUGFIX: do not drop `match[]` filter at [`/api/v1/series`](https://docs.victoriametrics.com/url-examples/#apiv1series) if `-search.ignoreExtraFiltersAtLabelsAPI` command-line flag is set, since missing `match[]` filter breaks `/api/v1/series` requests. * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl.html): properly parse TLS key and CA files for [InfluxDB](https://docs.victoriametrics.com/vmctl/#migrating-data-from-influxdb-1x) and [OpenTSDB](https://docs.victoriametrics.com/vmctl/#migrating-data-from-opentsdb) migration modes. * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): fix VictoriaLogs UI query handling to correctly apply `_time` filter across all queries. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5920). -* BUGFIX: [vmselect](https://docs.victoriametrics.com/): make vmselect resilient to absence of cache folder. If cache folder was mistakenly deleted by user or OS, vmselect will try re-creating it first. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5985). * BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): limit duration of requests to /api/v1/labels, /api/v1/label/.../values or /api/v1/series with `-search.maxLabelsAPIDuration` duration. Before, `-search.maxExportDuration` value was used by mistake. The bug has been introduced in [v1.99.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.99.0). Thanks to @kbweave for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5992). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): fix response body and headers for AWS Firehose HTTP Destination. * BUGFIX: properly wait for force merge to be completed during the shutdown. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5944) for the details. diff --git a/lib/fs/fs.go b/lib/fs/fs.go index 7775360c3..980066b10 100644 --- a/lib/fs/fs.go +++ b/lib/fs/fs.go @@ -351,7 +351,6 @@ func MustCreateFlockFile(dir string) *os.File { const FlockFilename = "flock.lock" // MustGetFreeSpace returns free space for the given directory path. -// It tries to re-create path if it doesn't exist yet. func MustGetFreeSpace(path string) uint64 { // Try obtaining cached value at first. freeSpaceMapLock.Lock() @@ -364,12 +363,6 @@ func MustGetFreeSpace(path string) uint64 { } // Slow path. - - // The path might be not available because: - // 1. We forgot to create it in the code - // 2. OS cleaned it up - MustMkdirIfNotExist(path) - // Determine the amount of free space at path. e.freeSpace = mustGetFreeSpace(path) e.updateTime = fasttime.UnixTimestamp()