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@xxxxxxxxxx> ## 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@xxxxxxxxxx> ## 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