[RFC PATCH v2 0/2] trace2: don't overload target directories
- From
Josh Steadmon <steadmon@google.com>
- Date
- Aug 2, 2019, 22:02 UTC
- Message-ID
- <cover.1564771000.git.steadmon@google.com>
- In-Reply-To
- <99e4a0fe409a236d210d95e54cd03fce61daa291.1564438745.git.steadmon@google.com>
I'm sending out V2 still as an RFC because I haven't yet had time to check that directory contention doesn't create problems with multiple processes sharing the same target directory. I'll be on vacation for the next couple of weeks, so I wanted to get the new config variable version out before then.
I noticed that git-config doesn't currently mention target directories, so I've included a patch to fix that as well.
Changes since V1: * Adds a new patch that includes a description of trace2's target-directory mode in the git-config documentation. * Moves the file count threshold from a #define constant to a config option. * Renames the threshold override envvar to be consistent with other trace2 envvars. * Simplified the new test case in t0210.
Josh Steadmon (2): docs: mention trace2 target-dir mode in git-config trace2: don't overload target directories
Documentation/config/trace2.txt | 6 ++ Documentation/technical/api-trace2.txt | 7 +-- Documentation/trace2-target-values.txt | 4 +- t/t0210-trace2-normal.sh | 19 ++++++ trace2/tr2_dst.c | 86 ++++++++++++++++++++++++++ trace2/tr2_sysenv.c | 3 + trace2/tr2_sysenv.h | 2 + 7 files changed, 122 insertions(+), 5 deletions(-)
Range-diff against v1:
-: ---------- > 1: 65e05a3db5 docs: mention trace2 target-dir mode in git-config
1: 99e4a0fe40 ! 2: a779e272df trace2: don't overload target directories
@@ Commit message
+ ## Documentation/config/trace2.txt ##
+@@ Documentation/config/trace2.txt: trace2.destinationDebug::
+ By default, these errors are suppressed and tracing is
+ silently disabled. May be overridden by the
+ `GIT_TRACE2_DST_DEBUG` environment variable.
++
++trace2.maxFiles::
++ Integer. When writing trace files to a target directory, do not
++ write additional traces if we would exceed this many files. Instead,
++ write a sentinel file that will block further tracing to this
++ directory. Defaults to 0, which disables this check.
+
## t/t0210-trace2-normal.sh ##
@@ t/t0210-trace2-normal.sh: test_expect_success 'using global config with include' '
test_cmp expect actual
'
+test_expect_success "don't overload target directory" '
-+ GIT_TRACE2_TEST_OVERLOAD_FILE_COUNT=100 &&
-+ export GIT_TRACE2_TEST_OVERLOAD_FILE_COUNT &&
-+ test_when_finished "rm -r trace_target_dir" &&
+ mkdir trace_target_dir &&
-+ test_seq $GIT_TRACE2_TEST_OVERLOAD_FILE_COUNT | sed "s#^#trace_target_dir/#" | sort > expected_filenames.txt &&
-+ xargs touch < expected_filenames.txt &&
-+ ls trace_target_dir | sed "s#^#trace_target_dir/#" > first_ls_output.txt &&
-+ test_cmp expected_filenames.txt first_ls_output.txt &&
-+ GIT_TRACE2="$(pwd)/trace_target_dir" test-tool trace2 001return 0 &&
-+ echo "trace_target_dir/git-trace2-overload" >> expected_filenames.txt &&
-+ ls trace_target_dir | sed "s#^#trace_target_dir/#" > second_ls_output.txt &&
++ test_when_finished "rm -r trace_target_dir" &&
++ (
++ GIT_TRACE2_MAX_FILES=5 &&
++ export GIT_TRACE2_MAX_FILES &&
++ cd trace_target_dir &&
++ test_seq $GIT_TRACE2_MAX_FILES >../expected_filenames.txt &&
++ xargs touch <../expected_filenames.txt &&
++ cd .. &&
++ ls trace_target_dir >first_ls_output.txt &&
++ test_cmp expected_filenames.txt first_ls_output.txt &&
++ GIT_TRACE2="$(pwd)/trace_target_dir" test-tool trace2 001return 0
++ ) &&
++ echo git-trace2-overload >>expected_filenames.txt &&
++ ls trace_target_dir >second_ls_output.txt &&
+ test_cmp expected_filenames.txt second_ls_output.txt
+'
+
@@ trace2/tr2_dst.c
+#define OVERLOAD_SENTINEL_NAME "git-trace2-overload"
+
+/*
-+ * How many files we can write to a directory before entering overload mode.
-+ * This can be overridden with the envvar GIT_TRACE2_TEST_OVERLOAD_FILE_COUNT
++ * When set to zero, disables directory overload checking. Otherwise, controls
++ * how many files we can write to a directory before entering overload mode.
++ * This can be overridden via the TR2_SYSENV_MAX_FILES setting.
+ */
-+#define OVERLOAD_FILE_COUNT 1000000
++static int tr2env_max_files = 0;
+
static int tr2_dst_want_warning(void)
{
@@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
+/*
+ * Check to make sure we're not overloading the target directory with too many
-+ * files. First check for the presence of a sentinel file, then check file
-+ * count. If we are overloaded, create the sentinel file if it doesn't already
-+ * exist.
++ * files. First get the threshold (if present) from the config or envvar. If
++ * it's zero or unset, disable this check. Next check for the presence of a
++ * sentinel file, then check file count. If we are overloaded, create the
++ * sentinel file if it doesn't already exist.
+ *
+ * We expect that some trace processing system is gradually collecting files
+ * from the target directory; after it removes the sentinel file we'll start
@@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
+ */
+static int tr2_dst_overloaded(const char *tgt_prefix)
+{
-+ int file_count = 0, overload_file_count = 0;
-+ char *test_threshold_val;
++ int file_count = 0, max_files = 0, ret = 0;
++ const char *max_files_var;
+ DIR *dirp;
+ struct strbuf path = STRBUF_INIT, sentinel_path = STRBUF_INIT;
+ struct stat statbuf;
@@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
+ strbuf_addch(&path, '/');
+ }
+
++ /* Get the config or envvar and decide if we should continue this check */
++ max_files_var = tr2_sysenv_get(TR2_SYSENV_MAX_FILES);
++ if (max_files_var && *max_files_var && ((max_files = atoi(max_files_var)) >= 0))
++ tr2env_max_files = max_files;
++
++ if (!tr2env_max_files) {
++ ret = 0;
++ goto cleanup;
++ }
++
+ /* check sentinel */
+ strbuf_addstr(&sentinel_path, path.buf);
+ strbuf_addstr(&sentinel_path, OVERLOAD_SENTINEL_NAME);
+ if (!stat(sentinel_path.buf, &statbuf)) {
-+ strbuf_release(&path);
-+ return 1;
++ ret = 1;
++ goto cleanup;
+ }
+
-+ /* check if we're overriding the threshold (e.g., for testing) */
-+ test_threshold_val = getenv("GIT_TRACE2_TEST_OVERLOAD_FILE_COUNT");
-+ if (test_threshold_val)
-+ overload_file_count = atoi(test_threshold_val);
-+ if (overload_file_count <= 0)
-+ overload_file_count = OVERLOAD_FILE_COUNT;
-+
-+
+ /* check file count */
+ dirp = opendir(path.buf);
-+ while (file_count < overload_file_count && dirp && readdir(dirp))
++ while (file_count < tr2env_max_files && dirp && readdir(dirp))
+ file_count++;
+ if (dirp)
+ closedir(dirp);
+
-+ if (file_count >= overload_file_count) {
++ if (file_count >= tr2env_max_files) {
+ creat(sentinel_path.buf, S_IRUSR | S_IWUSR);
-+ /* TODO: Write a target-specific message? */
-+ strbuf_release(&path);
-+ return 1;
++ ret = 1;
++ goto cleanup;
+ }
+
++cleanup:
+ strbuf_release(&path);
-+ return 0;
++ strbuf_release(&sentinel_path);
++ return ret;
+}
+
static int tr2_dst_try_auto_path(struct tr2_dst *dst, const char *tgt_prefix)
@@ trace2/tr2_dst.c: static int tr2_dst_try_auto_path(struct tr2_dst *dst, const ch
for (attempt_count = 0; attempt_count < MAX_AUTO_ATTEMPTS; attempt_count++) {
if (attempt_count > 0) {
strbuf_setlen(&path, base_path_len);
+
+ ## trace2/tr2_sysenv.c ##
+@@ trace2/tr2_sysenv.c: static struct tr2_sysenv_entry tr2_sysenv_settings[] = {
+ "trace2.perftarget" },
+ [TR2_SYSENV_PERF_BRIEF] = { "GIT_TRACE2_PERF_BRIEF",
+ "trace2.perfbrief" },
++
++ [TR2_SYSENV_MAX_FILES] = { "GIT_TRACE2_MAX_FILES",
++ "trace2.maxfiles" },
+ };
+ /* clang-format on */
+
+
+ ## trace2/tr2_sysenv.h ##
+@@ trace2/tr2_sysenv.h: enum tr2_sysenv_variable {
+ TR2_SYSENV_PERF,
+ TR2_SYSENV_PERF_BRIEF,
+
++ TR2_SYSENV_MAX_FILES,
++
+ TR2_SYSENV_MUST_BE_LAST
+ };
+
--
2.22.0.770.g0f2c4a37fd-goog