mirror of
https://github.com/VictoriaMetrics/VictoriaMetrics.git
synced 2024-11-21 14:44:00 +00:00
testing: allow disabling fsync to make tests run faster (#6871)
### Describe Your Changes fsync() ensures that the data is written to disk. In production this is needed for data durability. However, during the development, when the unit tests are run, this level of durability is not needed. Therefore fsync() can be disabled which will makes test runs two times faster. The disabling is done by setting the `DISABLE_FSYNC_FOR_TESTING` environment variable. The valid values for this variable are the same as the values of the arg of `go doc strconv.ParseBool`: ``` 1, t, T, TRUE, true, True, 0, f, F, FALSE, false, False. ``` Any other value means `false`. The variable is set for all test build targets. Compare running times: Build Target | DISABLE_FSYNC_FOR_TESTING=0 | DISABLE_FSYNC_FOR_TESTING=1 ----------------- | ------------------------------------------------ | ------------------------------------------------- make test | 1m5s | 0m22s make test-race | 3m1s | 1m42s make test-pure | 1m7s | 0m20s make test-full | 1m21s | 0m32s make test-full-386 | 1m42s | 0m36s When running tests for a given package, fsync can be disabled as follows: ```shell DISABLE_FSYNC_FOR_TESTING=1 go test ./lib/storage ``` Disabling fsync() is intended for testing purposes only and the name of the variables reflects that. What could also have been done but haven't: - lib/filestream/filestream.go: `Writer.MustFlush()` also uses f.Sync() but nothing has been done to it, because the Writer.MustFlush() is not used anywhere in the VM codebase. A side question: what is the general policy for the unused code? - lib/filestream/filestream.go: Writer.Write() calls `adviceDontNeed()` which calls unix.Fdatasync(). Disabling it could potentially improve running time, but running tests with this code disabled has shown otherwise. ### Checklist The following checks are **mandatory**: - [ x] My change adheres [VictoriaMetrics contributing guidelines](https://docs.victoriametrics.com/contributing/). --------- Signed-off-by: Artem Fetishev <wwctrsrx@gmail.com>
This commit is contained in:
parent
8a8a1d5df2
commit
334cd92a6c
8 changed files with 83 additions and 14 deletions
1
.gitignore
vendored
1
.gitignore
vendored
|
@ -26,3 +26,4 @@ _site
|
||||||
/docs/.jekyll-metadata
|
/docs/.jekyll-metadata
|
||||||
coverage.txt
|
coverage.txt
|
||||||
cspell.json
|
cspell.json
|
||||||
|
*~
|
||||||
|
|
12
Makefile
12
Makefile
|
@ -333,7 +333,7 @@ release-vmutils: \
|
||||||
|
|
||||||
release-vmutils-linux-386:
|
release-vmutils-linux-386:
|
||||||
GOOS=linux GOARCH=386 $(MAKE) release-vmutils-goos-goarch
|
GOOS=linux GOARCH=386 $(MAKE) release-vmutils-goos-goarch
|
||||||
|
|
||||||
release-vmutils-linux-amd64:
|
release-vmutils-linux-amd64:
|
||||||
GOOS=linux GOARCH=amd64 $(MAKE) release-vmutils-goos-goarch
|
GOOS=linux GOARCH=amd64 $(MAKE) release-vmutils-goos-goarch
|
||||||
|
|
||||||
|
@ -444,19 +444,19 @@ check-all: fmt vet golangci-lint govulncheck
|
||||||
clean-checkers: remove-golangci-lint remove-govulncheck
|
clean-checkers: remove-golangci-lint remove-govulncheck
|
||||||
|
|
||||||
test:
|
test:
|
||||||
go test ./lib/... ./app/...
|
DISABLE_FSYNC_FOR_TESTING=1 go test ./lib/... ./app/...
|
||||||
|
|
||||||
test-race:
|
test-race:
|
||||||
go test -race ./lib/... ./app/...
|
DISABLE_FSYNC_FOR_TESTING=1 go test -race ./lib/... ./app/...
|
||||||
|
|
||||||
test-pure:
|
test-pure:
|
||||||
CGO_ENABLED=0 go test ./lib/... ./app/...
|
DISABLE_FSYNC_FOR_TESTING=1 CGO_ENABLED=0 go test ./lib/... ./app/...
|
||||||
|
|
||||||
test-full:
|
test-full:
|
||||||
go test -coverprofile=coverage.txt -covermode=atomic ./lib/... ./app/...
|
DISABLE_FSYNC_FOR_TESTING=1 go test -coverprofile=coverage.txt -covermode=atomic ./lib/... ./app/...
|
||||||
|
|
||||||
test-full-386:
|
test-full-386:
|
||||||
GOARCH=386 go test -coverprofile=coverage.txt -covermode=atomic ./lib/... ./app/...
|
DISABLE_FSYNC_FOR_TESTING=1 GOARCH=386 go test -coverprofile=coverage.txt -covermode=atomic ./lib/... ./app/...
|
||||||
|
|
||||||
benchmark:
|
benchmark:
|
||||||
go test -bench=. ./lib/...
|
go test -bench=. ./lib/...
|
||||||
|
|
19
lib/envutil/envutil.go
Normal file
19
lib/envutil/envutil.go
Normal file
|
@ -0,0 +1,19 @@
|
||||||
|
package envutil
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"strconv"
|
||||||
|
)
|
||||||
|
|
||||||
|
// GetenvBool retrieves the value of the environment variable named by the key,
|
||||||
|
// attempts to convert the value to bool type and returns the result. In order
|
||||||
|
// for conversion to succeed, the value must be any value supported by
|
||||||
|
// strconv.ParseBool() function, otherwise the function will return false.
|
||||||
|
func GetenvBool(key string) bool {
|
||||||
|
s := os.Getenv(key)
|
||||||
|
b, err := strconv.ParseBool(s)
|
||||||
|
if err != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return b
|
||||||
|
}
|
37
lib/envutil/envutil_test.go
Normal file
37
lib/envutil/envutil_test.go
Normal file
|
@ -0,0 +1,37 @@
|
||||||
|
package envutil
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestGetenvBool(t *testing.T) {
|
||||||
|
f := func(value string, want bool) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
key := "VM_LIB_ENVUTIL_TEST_GETENV_BOOL"
|
||||||
|
os.Setenv(key, value)
|
||||||
|
defer os.Unsetenv(key)
|
||||||
|
|
||||||
|
if got := GetenvBool(key); got != want {
|
||||||
|
t.Errorf("GetenvBool(%s=%s) unexpected return value: got %t, want %t", key, value, got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
f("TRUE", true)
|
||||||
|
f("True", true)
|
||||||
|
f("true", true)
|
||||||
|
f("T", true)
|
||||||
|
f("t", true)
|
||||||
|
f("1", true)
|
||||||
|
f("FALSE", false)
|
||||||
|
f("False", false)
|
||||||
|
f("false", false)
|
||||||
|
f("F", false)
|
||||||
|
f("f", false)
|
||||||
|
f("0", false)
|
||||||
|
|
||||||
|
f("", false)
|
||||||
|
f("unsupported", false)
|
||||||
|
f("tRuE", false)
|
||||||
|
}
|
|
@ -9,6 +9,7 @@ import (
|
||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/VictoriaMetrics/VictoriaMetrics/lib/envutil"
|
||||||
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
|
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
|
||||||
"github.com/VictoriaMetrics/VictoriaMetrics/lib/memory"
|
"github.com/VictoriaMetrics/VictoriaMetrics/lib/memory"
|
||||||
"github.com/VictoriaMetrics/metrics"
|
"github.com/VictoriaMetrics/metrics"
|
||||||
|
@ -18,6 +19,8 @@ var disableFadvise = flag.Bool("filestream.disableFadvise", false, "Whether to d
|
||||||
"The fadvise() syscall prevents from eviction of recently accessed data from OS page cache during background merges and backups. "+
|
"The fadvise() syscall prevents from eviction of recently accessed data from OS page cache during background merges and backups. "+
|
||||||
"In some rare cases it is better to disable the syscall if it uses too much CPU")
|
"In some rare cases it is better to disable the syscall if it uses too much CPU")
|
||||||
|
|
||||||
|
var disableFSyncForTesting = envutil.GetenvBool("DISABLE_FSYNC_FOR_TESTING")
|
||||||
|
|
||||||
const dontNeedBlockSize = 16 * 1024 * 1024
|
const dontNeedBlockSize = 16 * 1024 * 1024
|
||||||
|
|
||||||
// ReadCloser is a standard interface for filestream Reader.
|
// ReadCloser is a standard interface for filestream Reader.
|
||||||
|
@ -241,8 +244,10 @@ func (w *Writer) MustClose() {
|
||||||
putBufioWriter(w.bw)
|
putBufioWriter(w.bw)
|
||||||
w.bw = nil
|
w.bw = nil
|
||||||
|
|
||||||
if err := w.f.Sync(); err != nil {
|
if !disableFSyncForTesting {
|
||||||
logger.Panicf("FATAL: cannot sync file %q: %d", w.f.Name(), err)
|
if err := w.f.Sync(); err != nil {
|
||||||
|
logger.Panicf("FATAL: cannot sync file %q: %d", w.f.Name(), err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if err := w.st.close(); err != nil {
|
if err := w.st.close(); err != nil {
|
||||||
logger.Panicf("FATAL: cannot close streamTracker for file %q: %s", w.f.Name(), err)
|
logger.Panicf("FATAL: cannot close streamTracker for file %q: %s", w.f.Name(), err)
|
||||||
|
|
|
@ -11,11 +11,14 @@ import (
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/VictoriaMetrics/VictoriaMetrics/lib/envutil"
|
||||||
"github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime"
|
"github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime"
|
||||||
"github.com/VictoriaMetrics/VictoriaMetrics/lib/filestream"
|
"github.com/VictoriaMetrics/VictoriaMetrics/lib/filestream"
|
||||||
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
|
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
var disableFSyncForTesting = envutil.GetenvBool("DISABLE_FSYNC_FOR_TESTING")
|
||||||
|
|
||||||
var tmpFileNum atomic.Uint64
|
var tmpFileNum atomic.Uint64
|
||||||
|
|
||||||
// MustSyncPath syncs contents of the given path.
|
// MustSyncPath syncs contents of the given path.
|
||||||
|
|
|
@ -30,9 +30,11 @@ func mustSyncPath(path string) {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
logger.Panicf("FATAL: cannot open file for fsync: %s", err)
|
logger.Panicf("FATAL: cannot open file for fsync: %s", err)
|
||||||
}
|
}
|
||||||
if err := d.Sync(); err != nil {
|
if !disableFSyncForTesting {
|
||||||
_ = d.Close()
|
if err := d.Sync(); err != nil {
|
||||||
logger.Panicf("FATAL: cannot flush %q to storage: %s", path, err)
|
_ = d.Close()
|
||||||
|
logger.Panicf("FATAL: cannot flush %q to storage: %s", path, err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if err := d.Close(); err != nil {
|
if err := d.Close(); err != nil {
|
||||||
logger.Panicf("FATAL: cannot close %q: %s", path, err)
|
logger.Panicf("FATAL: cannot close %q: %s", path, err)
|
||||||
|
|
|
@ -23,9 +23,11 @@ func mustSyncPath(path string) {
|
||||||
if err != nil {
|
if err != nil {
|
||||||
logger.Panicf("FATAL: cannot open file for fsync: %s", err)
|
logger.Panicf("FATAL: cannot open file for fsync: %s", err)
|
||||||
}
|
}
|
||||||
if err := d.Sync(); err != nil {
|
if !disableFSyncForTesting {
|
||||||
_ = d.Close()
|
if err := d.Sync(); err != nil {
|
||||||
logger.Panicf("FATAL: cannot flush %q to storage: %s", path, err)
|
_ = d.Close()
|
||||||
|
logger.Panicf("FATAL: cannot flush %q to storage: %s", path, err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if err := d.Close(); err != nil {
|
if err := d.Close(); err != nil {
|
||||||
logger.Panicf("FATAL: cannot close %q: %s", path, err)
|
logger.Panicf("FATAL: cannot close %q: %s", path, err)
|
||||||
|
|
Loading…
Reference in a new issue