git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Josh Steadmon <steadmon@google.com>
To: git@vger.kernel.org
Cc: stolee@gmail.com, git@jeffhostetler.com
Subject: [RFC PATCH v2 0/2] trace2: don't overload target directories
Date: Fri,  2 Aug 2019 15:02:33 -0700	[thread overview]
Message-ID: <cover.1564771000.git.steadmon@google.com> (raw)
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


  parent reply	other threads:[~2019-08-02 22:02 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-29 22:20 [RFC PATCH] " Josh Steadmon
2019-07-30 13:29 ` Derrick Stolee
2019-07-30 21:52   ` Josh Steadmon
2019-07-30 16:46 ` Jeff Hostetler
2019-07-30 22:01   ` Josh Steadmon
2019-07-30 22:02   ` Josh Steadmon
2019-07-30 18:00 ` Jeff Hostetler
2019-07-30 22:08   ` Josh Steadmon
2019-08-02 22:02 ` Josh Steadmon [this message]
2019-08-02 22:02   ` [RFC PATCH v2 1/2] docs: mention trace2 target-dir mode in git-config Josh Steadmon
2019-08-02 22:02   ` [RFC PATCH v2 2/2] trace2: don't overload target directories Josh Steadmon
2019-08-05 15:34     ` Jeff Hostetler
2019-08-05 18:17       ` Josh Steadmon
2019-08-05 18:01     ` SZEDER Gábor
2019-08-05 18:09       ` Josh Steadmon
2019-09-14  0:25 ` [RFC PATCH v3 0/3] " Josh Steadmon
2019-09-14  0:25   ` [RFC PATCH v3 1/3] docs: mention trace2 target-dir mode in git-config Josh Steadmon
2019-09-14  0:25   ` [RFC PATCH v3 2/3] trace2: don't overload target directories Josh Steadmon
2019-09-14  0:26   ` [RFC PATCH v3 3/3] trace2: write overload message to sentinel files Josh Steadmon
2019-09-16 12:07     ` Derrick Stolee
2019-09-16 14:11       ` Jeff Hostetler
2019-09-16 18:20         ` Josh Steadmon
2019-09-19 18:23           ` Jeff Hostetler
2019-09-19 22:47             ` Josh Steadmon
2019-09-20 15:59               ` Jeff Hostetler
2019-09-16 18:07       ` Josh Steadmon
2019-10-03 23:32 ` [PATCH v4 0/4] trace2: don't overload target directories Josh Steadmon
2019-10-03 23:32   ` [PATCH v4 1/4] docs: mention trace2 target-dir mode in git-config Josh Steadmon
2019-10-03 23:32   ` [PATCH v4 2/4] docs: clarify trace2 version invariants Josh Steadmon
2019-10-03 23:32   ` [PATCH v4 3/4] trace2: don't overload target directories Josh Steadmon
2019-10-04  0:25     ` Junio C Hamano
2019-10-04 21:57       ` Josh Steadmon
2019-10-04  9:12     ` Johannes Schindelin
2019-10-04 22:05       ` Josh Steadmon
2019-10-03 23:32   ` [PATCH v4 4/4] trace2: write overload message to sentinel files Josh Steadmon
2019-10-04 22:08 ` [PATCH v5 0/4] trace2: discard new traces if the target directory contains too many files Josh Steadmon
2019-10-04 22:08   ` [PATCH v5 1/4] docs: mention trace2 target-dir mode in git-config Josh Steadmon
2019-10-04 22:08   ` [PATCH v5 2/4] docs: clarify trace2 version invariants Josh Steadmon
2019-10-04 22:08   ` [PATCH v5 3/4] trace2: discard new traces if target directory has too many files Josh Steadmon
2019-10-04 22:08   ` [PATCH v5 4/4] trace2: write discard message to sentinel files Josh Steadmon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1564771000.git.steadmon@google.com \
    --to=steadmon@google.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=stolee@gmail.com \
    --subject='Re: [RFC PATCH v2 0/2] trace2: don'\''t overload target directories' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).