[v4,0/4] trace2: don't overload target directories

Message

Josh Steadmon Oct. 3, 2019, 11:32 p.m. UTC

Apologies for the delayed reply. I've added a new documentation patch
describing what we expect w.r.t. the version event and its "evt" field.
I've also reworked the final patch to allow writing a full trace session
to the sentinel file, and to make sure that the overload event comes
after the version event.

I am still incrementing the EVT and writing a new "overload" event,
since you said in the last review that this approache seems best to you.
But I am also happy to keep the EVT=1 and make "overload" a new field in
the "version" event if you've changed your mind.

Josh Steadmon (4):
  docs: mention trace2 target-dir mode in git-config
  docs: clarify trace2 version invariants
  trace2: don't overload target directories
  trace2: write overload message to sentinel files

 Documentation/config/trace2.txt        |   6 ++
 Documentation/technical/api-trace2.txt |  30 +++++--
 Documentation/trace2-target-values.txt |   4 +-
 t/t0212-trace2-event.sh                |  19 +++++
 trace2/tr2_dst.c                       | 113 ++++++++++++++++++++++---
 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, 185 insertions(+), 28 deletions(-)

Range-diff against v3:
-:  ---------- > 1:  a757bca8f9 docs: clarify trace2 version invariants
1:  bf20ec8ea2 ! 2:  98a8440d3f trace2: don't overload target directories
    @@ Commit message
         The config can also be overridden with a new environment variable:
         GIT_TRACE2_MAX_FILES.
     
    -    Potential future work:
    -    * Write a message into the sentinel file (should match the requested
    -      trace2 output format).
    -    * Add a performance test to make sure that contention between multiple
    -      processes all writing to the same target directory does not become an
    -      issue.
    -
     
      ## Documentation/config/trace2.txt ##
    @@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
     +	struct strbuf path = STRBUF_INIT, sentinel_path = STRBUF_INIT;
     +	struct stat statbuf;
     +
    -+	strbuf_addstr(&path, tgt_prefix);
    -+	if (!is_dir_sep(path.buf[path.len - 1])) {
    -+		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))
    @@ trace2/tr2_dst.c: void tr2_dst_trace_disable(struct tr2_dst *dst)
     +		goto cleanup;
     +	}
     +
    ++	strbuf_addstr(&path, tgt_prefix);
    ++	if (!is_dir_sep(path.buf[path.len - 1])) {
    ++		strbuf_addch(&path, '/');
    ++	}
    ++
     +	/* check sentinel */
     +	strbuf_addbuf(&sentinel_path, &path);
     +	strbuf_addstr(&sentinel_path, OVERLOAD_SENTINEL_NAME);
2:  bab45cb735 < -:  ---------- trace2: write overload message to sentinel files
-:  ---------- > 3:  790c5ac95a trace2: write overload message to sentinel files