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
3 years ago · e03d958b91
parent 00889cf8f2
commit e03d958b91
5 changed files with 43 additions and 78 deletions
--- a/75
+++ b/75
@ -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 <duration,pass/fail,name>,'		\
 	  '  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 <duration,pass/fail,name>,'		\
 	  '  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

--- a/common.mk
+++ 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
--- 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)
--- a/python.mk
+++ b/python.mk
@ -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
--- a/test_util/testutil.h
+++ b/test_util/testutil.h
@ -819,11 +819,6 @@ bool IsPrefetchSupported(const std::shared_ptr<FileSystem>& 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);