[PATCH v5 0/4] trace2: discard new traces if the target directory contains too many files

[Josh Steadmon <steadmon@google.com>]

Changes since V4:
* Avoid the use of the non-specific term "overload" in code, trace
  output, commit messages, and documentation.
* Remove an unnecessary <dirent.h> include that was breaking the Windows
  build.

Josh Steadmon (4):
  docs: mention trace2 target-dir mode in git-config
  docs: clarify trace2 version invariants
  trace2: discard new traces if target directory has too many files
  trace2: write discard message to sentinel files

 Documentation/config/trace2.txt        |   6 ++
 Documentation/technical/api-trace2.txt |  31 +++++--
 Documentation/trace2-target-values.txt |   4 +-
 t/t0212-trace2-event.sh                |  19 +++++
 trace2/tr2_dst.c                       | 111 ++++++++++++++++++++++---
 trace2/tr2_dst.h                       |   1 +
 trace2/tr2_sysenv.c                    |   3 +
 trace2/tr2_sysenv.h                    |   2 +
 trace2/tr2_tgt_event.c                 |  31 +++++--
 trace2/tr2_tgt_normal.c                |   2 +-
 trace2/tr2_tgt_perf.c                  |   2 +-
 11 files changed, 184 insertions(+), 28 deletions(-)

Range-diff against v4:
1:  98a8440d3f ! 1:  391051b308 trace2: don't overload target directories
    @@ Metadata
     Author: Josh Steadmon <steadmon@google.com>
     
      ## Commit message ##
    -    trace2: don't overload target directories
    +    trace2: discard new traces if target directory has too many files
     
         trace2 can write files into a target directory. With heavy usage, this
         directory can fill up with files, causing difficulty for
    @@ Commit message
         following behavior is enabled when the maxFiles is set to a positive
         integer:
           When trace2 would write a file to a target directory, first check
    -      whether or not the directory is overloaded. A directory is overloaded
    -      if there is a sentinel file declaring an overload, or if the number of
    -      files exceeds trace2.maxFiles. If the latter, create a sentinel file
    -      to speed up later overload checks.
    +      whether or not the traces should be discarded. Traces should be
    +      discarded if:
    +        * there is a sentinel file declaring that there are too many files
    +        * OR, the number of files exceeds trace2.maxFiles.
    +      In the latter case, we create a sentinel file named git-trace2-discard
    +      to speed up future checks.
     
         The assumption is that a separate trace-processing system is dealing
         with the generated traces; once it processes and removes the sentinel
         file, it should be safe to generate new trace files again.
     
    -    The default value for trace2.maxFiles is zero, which disables the
    -    overload check.
    +    The default value for trace2.maxFiles is zero, which disables the file
    +    count check.
     
         The config can also be overridden with a new environment variable:
         GIT_TRACE2_MAX_FILES.
    @@ t/t0212-trace2-event.sh: test_expect_success JSON_PP 'using global config, event
      	test_cmp expect actual
      '
      
    -+test_expect_success "don't overload target directory" '
    ++test_expect_success 'discard traces when there are too many files' '
     +	mkdir trace_target_dir &&
     +	test_when_finished "rm -r trace_target_dir" &&
     +	(
    @@ t/t0212-trace2-event.sh: test_expect_success JSON_PP 'using global config, event
     +		cd .. &&
     +		GIT_TRACE2_EVENT="$(pwd)/trace_target_dir" test-tool trace2 001return 0
     +	) &&
    -+	echo git-trace2-overload >>expected_filenames.txt &&
    ++	echo git-trace2-discard >>expected_filenames.txt &&
     +	ls trace_target_dir >ls_output.txt &&
     +	test_cmp expected_filenames.txt ls_output.txt
     +'
    @@ t/t0212-trace2-event.sh: test_expect_success JSON_PP 'using global config, event
      test_done
     
      ## trace2/tr2_dst.c ##
    -@@
    -+#include <dirent.h>
    -+
    - #include "cache.h"
    - #include "trace2/tr2_dst.h"
    - #include "trace2/tr2_sid.h"
     @@
       */
      #define MAX_AUTO_ATTEMPTS 10
      
     +/*
    -+ * Sentinel file used to detect when we're overloading a directory with too many
    -+ * trace files.
    ++ * Sentinel file used to detect when we should discard new traces to avoid
    ++ * writing too many trace files to a directory.
     + */
    -+#define OVERLOAD_SENTINEL_NAME "git-trace2-overload"
    ++#define DISCARD_SENTINEL_NAME "git-trace2-discard"
     +
     +/*
    -+ * When set to zero, disables directory overload checking. Otherwise, controls
    -+ * how many files we can write to a directory before entering overload mode.
    ++ * When set to zero, disables directory file count checks. Otherwise, controls
    ++ * how many files we can write to a directory before entering discard mode.
     + * This can be overridden via the TR2_SYSENV_MAX_FILES setting.
     + */
     +static int tr2env_max_files = 0;
    @@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
     + * from the target directory; after it removes the sentinel file we'll start
     + * writing traces again.
     + */
    -+static int tr2_dst_overloaded(const char *tgt_prefix)
    ++static int tr2_dst_too_many_files(const char *tgt_prefix)
     +{
     +	int file_count = 0, max_files = 0, ret = 0;
     +	const char *max_files_var;
    @@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
     +
     +	/* check sentinel */
     +	strbuf_addbuf(&sentinel_path, &path);
    -+	strbuf_addstr(&sentinel_path, OVERLOAD_SENTINEL_NAME);
    ++	strbuf_addstr(&sentinel_path, DISCARD_SENTINEL_NAME);
     +	if (!stat(sentinel_path.buf, &statbuf)) {
     +		ret = 1;
     +		goto cleanup;
    @@ trace2/tr2_dst.c: static int tr2_dst_try_auto_path(struct tr2_dst *dst, const ch
      	strbuf_addstr(&path, sid);
      	base_path_len = path.len;
      
    -+	if (tr2_dst_overloaded(tgt_prefix)) {
    ++	if (tr2_dst_too_many_files(tgt_prefix)) {
     +		strbuf_release(&path);
     +		if (tr2_dst_want_warning())
     +			warning("trace2: not opening %s trace file due to too "
2:  790c5ac95a ! 2:  1c3c7f01c6 trace2: write overload message to sentinel files
    @@ Metadata
     Author: Josh Steadmon <steadmon@google.com>
     
      ## Commit message ##
    -    trace2: write overload message to sentinel files
    +    trace2: write discard message to sentinel files
     
    -    Add a new "overload" event type for trace2 event destinations. When the
    -    trace2 overload feature creates a sentinel file, it will include the
    -    normal trace2 output in the sentinel, along with this new overload
    +    Add a new "discard" event type for trace2 event destinations. When the
    +    trace2 file count check creates a sentinel file, it will include the
    +    normal trace2 output in the sentinel, along with this new discard
         event.
     
         Writing this message into the sentinel file is useful for tracking how
    -    often the overload protection feature is triggered in practice.
    +    often the file count check triggers in practice.
     
         Bump up the event format version since we've added a new event type.
     
    @@ Documentation/technical/api-trace2.txt: only present on the "start" and "atexit"
      }
      ------------
      
    -+`"overload"`::
    -+	This event is created in a sentinel file if we are overloading a target
    -+	trace directory (see the trace2.maxFiles config option).
    ++`"discard"`::
    ++	This event is written to the git-trace2-discard sentinel file if there
    ++	are too many files in the target trace directory (see the
    ++	trace2.maxFiles config option).
     ++
     +------------
     +{
    -+	"event":"overload",
    ++	"event":"discard",
     +	...
     +}
     +------------
    @@ Documentation/technical/api-trace2.txt: only present on the "start" and "atexit"
      +
     
      ## t/t0212-trace2-event.sh ##
    -@@ t/t0212-trace2-event.sh: test_expect_success "don't overload target directory" '
    +@@ t/t0212-trace2-event.sh: test_expect_success 'discard traces when there are too many files' '
      	) &&
    - 	echo git-trace2-overload >>expected_filenames.txt &&
    + 	echo git-trace2-discard >>expected_filenames.txt &&
      	ls trace_target_dir >ls_output.txt &&
     -	test_cmp expected_filenames.txt ls_output.txt
     +	test_cmp expected_filenames.txt ls_output.txt &&
    -+	head -n1 trace_target_dir/git-trace2-overload | grep \"event\":\"version\" &&
    -+	head -n2 trace_target_dir/git-trace2-overload | tail -n1 | grep \"event\":\"overload\"
    ++	head -n1 trace_target_dir/git-trace2-discard | grep \"event\":\"version\" &&
    ++	head -n2 trace_target_dir/git-trace2-discard | tail -n1 | grep \"event\":\"too_many_files\"
      '
      
      test_done
    @@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
     + * sentinel file, then check file count.
     + *
     + * Returns 0 if tracing should proceed as normal. Returns 1 if the sentinel file
    -+ * already exists, which means tracing should be disabled. Returns -1 if we are
    -+ * overloaded but there was no sentinel file, which means we have created and
    -+ * should write traces to the sentinel file.
    ++ * already exists, which means tracing should be disabled. Returns -1 if there
    ++ * are too many files but there was no sentinel file, which means we have
    ++ * created and should write traces to the sentinel file.
       *
       * We expect that some trace processing system is gradually collecting files
       * from the target directory; after it removes the sentinel file we'll start
       * writing traces again.
       */
    --static int tr2_dst_overloaded(const char *tgt_prefix)
    -+static int tr2_dst_overloaded(struct tr2_dst *dst, const char *tgt_prefix)
    +-static int tr2_dst_too_many_files(const char *tgt_prefix)
    ++static int tr2_dst_too_many_files(struct tr2_dst *dst, const char *tgt_prefix)
      {
      	int file_count = 0, max_files = 0, ret = 0;
      	const char *max_files_var;
    -@@ trace2/tr2_dst.c: static int tr2_dst_overloaded(const char *tgt_prefix)
    +@@ trace2/tr2_dst.c: static int tr2_dst_too_many_files(const char *tgt_prefix)
      		closedir(dirp);
      
      	if (file_count >= tr2env_max_files) {
     -		creat(sentinel_path.buf, 0666);
     -		ret = 1;
    -+		dst->overloaded = 1;
    ++		dst->too_many_files = 1;
     +		dst->fd = open(sentinel_path.buf, O_WRONLY | O_CREAT | O_EXCL, 0666);
     +		ret = -1;
      		goto cleanup;
      	}
      
    -@@ trace2/tr2_dst.c: static int tr2_dst_overloaded(const char *tgt_prefix)
    +@@ trace2/tr2_dst.c: static int tr2_dst_too_many_files(const char *tgt_prefix)
      
      static int tr2_dst_try_auto_path(struct tr2_dst *dst, const char *tgt_prefix)
      {
     -	int fd;
    -+	int overloaded;
    ++	int too_many_files;
      	const char *last_slash, *sid = tr2_sid_get();
      	struct strbuf path = STRBUF_INIT;
      	size_t base_path_len;
    @@ trace2/tr2_dst.c: static int tr2_dst_try_auto_path(struct tr2_dst *dst, const ch
      	strbuf_addstr(&path, sid);
      	base_path_len = path.len;
      
    --	if (tr2_dst_overloaded(tgt_prefix)) {
    -+	overloaded = tr2_dst_overloaded(dst, tgt_prefix);
    -+	if (!overloaded) {
    +-	if (tr2_dst_too_many_files(tgt_prefix)) {
    ++	too_many_files = tr2_dst_too_many_files(dst, tgt_prefix);
    ++	if (!too_many_files) {
     +		for (attempt_count = 0; attempt_count < MAX_AUTO_ATTEMPTS; attempt_count++) {
     +			if (attempt_count > 0) {
     +				strbuf_setlen(&path, base_path_len);
    @@ trace2/tr2_dst.c: static int tr2_dst_try_auto_path(struct tr2_dst *dst, const ch
     +			if (dst->fd != -1)
     +				break;
     +		}
    -+	} else if (overloaded == 1) {
    ++	} else if (too_many_files == 1) {
      		strbuf_release(&path);
      		if (tr2_dst_want_warning())
      			warning("trace2: not opening %s trace file due to too "
    @@ trace2/tr2_dst.h: struct tr2_dst {
      	int fd;
      	unsigned int initialized : 1;
      	unsigned int need_close : 1;
    -+	unsigned int overloaded : 1;
    ++	unsigned int too_many_files : 1;
      };
      
      /*
    @@ trace2/tr2_tgt_event.c: static void event_fmt_prepare(const char *event_name, co
      		jw_object_intmax(jw, "repo", repo->trace2_repo_id);
      }
      
    -+static void fn_overload_fl(const char *file, int line)
    ++static void fn_too_many_files_fl(const char *file, int line)
     +{
    -+	const char *event_name = "overload";
    ++	const char *event_name = "too_many_files";
     +	struct json_writer jw = JSON_WRITER_INIT;
     +
     +	jw_object_begin(&jw, 0);
    @@ trace2/tr2_tgt_event.c: static void fn_version_fl(const char *file, int line)
      	tr2_dst_write_line(&tr2dst_event, &jw.json);
      	jw_release(&jw);
     +
    -+	if (tr2dst_event.overloaded)
    -+		fn_overload_fl(file, line);
    ++	if (tr2dst_event.too_many_files)
    ++		fn_too_many_files_fl(file, line);
      }
      
      static void fn_start_fl(const char *file, int line,
-- 
2.23.0.581.g78d2f28ef7-goog