From e03d958b9166de6f29288f2ed854722b36cd3e0e Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Fri, 6 May 2022 16:38:06 -0700 Subject: [PATCH] Clean up variables for temporary directory (#9961) Summary: Having all of TMPD, TMPDIR and TEST_TMPDIR as configuration parameters is confusing. This change simplifies a number of things by standardizing on TEST_TMPDIR, while still recognizing the old names also. In detail: * crash_test.mk also needs to use TEST_TMPDIR for crash test, so put in shared common.mk (an upgrade of python.mk) * Always exporting TEST_TMPDIR eliminates the need to propagate TMPD or export TEST_TMPDIR in selective places. * Use --tmpdir option to gnu_parallel so that it doesn't need TMPDIR environment variable * Remove obsolete parloop and parallel_check Makefile targets * Remove undefined, unused function ResetTmpDirForDirectIO() Pull Request resolved: https://github.com/facebook/rocksdb/pull/9961 Test Plan: manual + CI Reviewed By: riversand963 Differential Revision: D36212178 Pulled By: pdillinger fbshipit-source-id: b76c1876c4f4d38b37789c2779eaa7c3026824dd --- Makefile | 75 +++++++------------------------------------- common.mk | 30 ++++++++++++++++++ crash_test.mk | 2 +- python.mk | 9 ------ test_util/testutil.h | 5 --- 5 files changed, 43 insertions(+), 78 deletions(-) create mode 100644 common.mk delete mode 100644 python.mk diff --git a/Makefile b/Makefile index c11b98ca7..93a67b654 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ BASH_EXISTS := $(shell which bash) SHELL := $(shell which bash) -include python.mk +include common.mk CLEAN_FILES = # deliberately empty, so we can append below. CFLAGS += ${EXTRA_CFLAGS} @@ -842,18 +842,6 @@ coverage: clean # Delete intermediate files $(FIND) . -type f \( -name "*.gcda" -o -name "*.gcno" \) -exec rm -f {} \; -ifneq (,$(filter check parallel_check,$(MAKECMDGOALS)),) -# Use /dev/shm if it has the sticky bit set (otherwise, /tmp), -# and create a randomly-named rocksdb.XXXX directory therein. -# We'll use that directory in the "make check" rules. -ifeq ($(TMPD),) -TMPDIR := $(shell echo $${TMPDIR:-/tmp}) -TMPD := $(shell f=/dev/shm; test -k $$f || f=$(TMPDIR); \ - perl -le 'use File::Temp "tempdir";' \ - -e 'print tempdir("'$$f'/rocksdb.XXXX", CLEANUP => 0)') -endif -endif - # Run all tests in parallel, accumulating per-test logs in t/log-*. # # Each t/run-* file is a tiny generated bourne shell script that invokes one of @@ -893,7 +881,7 @@ $(parallel_tests): TEST_SCRIPT=t/run-$$TEST_BINARY-$${TEST_NAME//\//-}; \ printf '%s\n' \ '#!/bin/sh' \ - "d=\$(TMPD)$$TEST_SCRIPT" \ + "d=\$(TEST_TMPDIR)$$TEST_SCRIPT" \ 'mkdir -p $$d' \ "TEST_TMPDIR=\$$d $(DRIVER) ./$$TEST_BINARY --gtest_filter=$$TEST_NAME" \ > $$TEST_SCRIPT; \ @@ -953,7 +941,6 @@ endif .PHONY: check_0 check_0: - $(AM_V_GEN)export TEST_TMPDIR=$(TMPD); \ printf '%s\n' '' \ 'To monitor subtest ,' \ ' run "make watch-log" in a separate window' ''; \ @@ -964,7 +951,8 @@ check_0: | $(prioritize_long_running_tests) \ | grep -E '$(tests-regexp)' \ | grep -E -v '$(EXCLUDE_TESTS_REGEX)' \ - | build_tools/gnu_parallel -j$(J) --plain --joblog=LOG --eta --gnu '{} $(parallel_redir)' ; \ + | build_tools/gnu_parallel -j$(J) --plain --joblog=LOG --eta --gnu \ + --tmpdir=$(TEST_TMPDIR) '{} $(parallel_redir)' ; \ parallel_retcode=$$? ; \ awk '{ if ($$7 != 0 || $$8 != 0) { if ($$7 == "Exitval") { h = $$0; } else { if (!f) print h; print; f = 1 } } } END { if(f) exit 1; }' < LOG ; \ awk_retcode=$$?; \ @@ -975,7 +963,6 @@ valgrind-exclude-regexp = InlineSkipTest.ConcurrentInsert|TransactionStressTest. .PHONY: valgrind_check_0 valgrind_check_0: test_log_prefix := valgrind_ valgrind_check_0: - $(AM_V_GEN)export TEST_TMPDIR=$(TMPD); \ printf '%s\n' '' \ 'To monitor subtest ,' \ ' run "make watch-log" in a separate window' ''; \ @@ -987,10 +974,11 @@ valgrind_check_0: | grep -E '$(tests-regexp)' \ | grep -E -v '$(valgrind-exclude-regexp)' \ | build_tools/gnu_parallel -j$(J) --plain --joblog=LOG --eta --gnu \ - '(if [[ "{}" == "./"* ]] ; then $(DRIVER) {}; else {}; fi) \ + --tmpdir=$(TEST_TMPDIR) \ + '(if [[ "{}" == "./"* ]] ; then $(DRIVER) {}; else {}; fi) \ $(parallel_redir)' \ -CLEAN_FILES += t LOG $(TMPD) +CLEAN_FILES += t LOG $(TEST_TMPDIR) # When running parallel "make check", you can monitor its progress # from another window. @@ -1013,12 +1001,12 @@ check: all && (build_tools/gnu_parallel --gnu --help 2>/dev/null) | \ grep -q 'GNU Parallel'; \ then \ - $(MAKE) T="$$t" TMPD=$(TMPD) check_0; \ + $(MAKE) T="$$t" check_0; \ else \ for t in $(TESTS); do \ echo "===== Running $$t (`date`)"; ./$$t || exit 1; done; \ fi - rm -rf $(TMPD) + rm -rf $(TEST_TMPDIR) ifneq ($(PLATFORM), OS_AIX) $(PYTHON) tools/check_all_python.py ifeq ($(filter -DROCKSDB_LITE,$(OPT)),) @@ -1115,11 +1103,11 @@ valgrind_test_some: valgrind_check: $(TESTS) $(MAKE) DRIVER="$(VALGRIND_VER) $(VALGRIND_OPTS)" gen_parallel_tests $(AM_V_GEN)if test "$(J)" != 1 \ - && (build_tools/gnu_parallel --gnu --help 2>/dev/null) | \ + && (build_tools/gnu_parallel --gnu --help 2>/dev/null) | \ grep -q 'GNU Parallel'; \ then \ - $(MAKE) TMPD=$(TMPD) \ - DRIVER="$(VALGRIND_VER) $(VALGRIND_OPTS)" valgrind_check_0; \ + $(MAKE) \ + DRIVER="$(VALGRIND_VER) $(VALGRIND_OPTS)" valgrind_check_0; \ else \ for t in $(filter-out %skiplist_test options_settable_test,$(TESTS)); do \ $(VALGRIND_VER) $(VALGRIND_OPTS) ./$$t; \ @@ -1139,27 +1127,6 @@ valgrind_check_some: $(ROCKSDBTESTS_SUBSET) fi; \ done -ifneq ($(PAR_TEST),) -parloop: - ret_bad=0; \ - for t in $(PAR_TEST); do \ - echo "===== Running $$t in parallel $(NUM_PAR) (`date`)";\ - if [ $(db_test) -eq 1 ]; then \ - seq $(J) | v="$$t" build_tools/gnu_parallel --gnu --plain 's=$(TMPD)/rdb-{}; export TEST_TMPDIR=$$s;' \ - 'timeout 2m ./db_test --gtest_filter=$$v >> $$s/log-{} 2>1'; \ - else\ - seq $(J) | v="./$$t" build_tools/gnu_parallel --gnu --plain 's=$(TMPD)/rdb-{};' \ - 'export TEST_TMPDIR=$$s; timeout 10m $$v >> $$s/log-{} 2>1'; \ - fi; \ - ret_code=$$?; \ - if [ $$ret_code -ne 0 ]; then \ - ret_bad=$$ret_code; \ - echo $$t exited with $$ret_code; \ - fi; \ - done; \ - exit $$ret_bad; -endif - test_names = \ ./db_test --gtest_list_tests \ | perl -n \ @@ -1167,24 +1134,6 @@ test_names = \ -e '/^(\s*)(\S+)/; !$$1 and do {$$p=$$2; break};' \ -e 'print qq! $$p$$2!' -parallel_check: $(TESTS) - $(AM_V_GEN)if test "$(J)" > 1 \ - && (build_tools/gnu_parallel --gnu --help 2>/dev/null) | \ - grep -q 'GNU Parallel'; \ - then \ - echo Running in parallel $(J); \ - else \ - echo "Need to have GNU Parallel and J > 1"; exit 1; \ - fi; \ - ret_bad=0; \ - echo $(J);\ - echo Test Dir: $(TMPD); \ - seq $(J) | build_tools/gnu_parallel --gnu --plain 's=$(TMPD)/rdb-{}; rm -rf $$s; mkdir $$s'; \ - $(MAKE) PAR_TEST="$(shell $(test_names))" TMPD=$(TMPD) \ - J=$(J) db_test=1 parloop; \ - $(MAKE) PAR_TEST="$(filter-out db_test, $(TESTS))" \ - TMPD=$(TMPD) J=$(J) db_test=0 parloop; - analyze: clean USE_CLANG=1 $(MAKE) analyze_incremental diff --git a/common.mk b/common.mk new file mode 100644 index 000000000..85c99fcec --- /dev/null +++ b/common.mk @@ -0,0 +1,30 @@ +ifndef PYTHON + +# Default to python3. Some distros like CentOS 8 do not have `python`. +ifeq ($(origin PYTHON), undefined) + PYTHON := $(shell which python3 || which python || echo python3) +endif +export PYTHON + +endif + +# To setup tmp directory, first recognize some old variables for setting +# test tmp directory or base tmp directory. TEST_TMPDIR is usually read +# by RocksDB tools though Env/FileSystem::GetTestDirectory. +ifeq ($(TEST_TMPDIR),) +TEST_TMPDIR := $(TMPD) +endif +ifeq ($(TEST_TMPDIR),) +ifeq ($(BASE_TMPDIR),) +BASE_TMPDIR :=$(TMPDIR) +endif +ifeq ($(BASE_TMPDIR),) +BASE_TMPDIR :=/tmp +endif +# Use /dev/shm if it has the sticky bit set (otherwise, /tmp or other +# base dir), and create a randomly-named rocksdb.XXXX directory therein. +TEST_TMPDIR := $(shell f=/dev/shm; test -k $$f || f=$(BASE_TMPDIR); \ + perl -le 'use File::Temp "tempdir";' \ + -e 'print tempdir("'$$f'/rocksdb.XXXX", CLEANUP => 0)') +endif +export TEST_TMPDIR diff --git a/crash_test.mk b/crash_test.mk index f389c1815..2a6a1f308 100644 --- a/crash_test.mk +++ b/crash_test.mk @@ -5,7 +5,7 @@ # build DB_STRESS_CMD so it must exist prior. DB_STRESS_CMD?=./db_stress -include python.mk +include common.mk CRASHTEST_MAKE=$(MAKE) -f crash_test.mk CRASHTEST_PY=$(PYTHON) -u tools/db_crashtest.py --stress_cmd=$(DB_STRESS_CMD) diff --git a/python.mk b/python.mk deleted file mode 100644 index d92cdec47..000000000 --- a/python.mk +++ /dev/null @@ -1,9 +0,0 @@ -ifndef PYTHON - -# Default to python3. Some distros like CentOS 8 do not have `python`. -ifeq ($(origin PYTHON), undefined) - PYTHON := $(shell which python3 || which python || echo python3) -endif -export PYTHON - -endif diff --git a/test_util/testutil.h b/test_util/testutil.h index 712862f2e..1fc454dcd 100644 --- a/test_util/testutil.h +++ b/test_util/testutil.h @@ -819,11 +819,6 @@ bool IsPrefetchSupported(const std::shared_ptr& fs, // Return the number of lines where a given pattern was found in a file. size_t GetLinesCount(const std::string& fname, const std::string& pattern); -// TEST_TMPDIR may be set to /dev/shm in Makefile, -// but /dev/shm does not support direct IO. -// Tries to set TEST_TMPDIR to a directory supporting direct IO. -void ResetTmpDirForDirectIO(); - Status CorruptFile(Env* env, const std::string& fname, int offset, int bytes_to_corrupt, bool verify_checksum = true); Status TruncateFile(Env* env, const std::string& fname, uint64_t length);