Re: [PATCH v4 3/4] trace2: don't overload target directories

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 4, 2019, 00:25 UTC
Message-ID: <xmqq1rvt9x13.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <98a8440d3f0ef3cd3cdd0291051d976d4a659cc4.1570144820.git.steadmon@google.com>

Josh Steadmon <steadmon@google.com> writes:

> trace2 can write files into a target directory. With heavy usage, this
> directory can fill up with files, causing difficulty for
> trace-processing systems.

Sorry for not mentioning this, but "don't overload" is a suboptimal keyword for the entire topic and for this particular step for a few reasons. For one, "overload" is an overloaded verb that gives an incorrect impression that the problem you are dealing with is that the target directory you specify is (mis)used for other purposes, which is not the case. You instead refrain from creating too many files. The other (which is probably more serious) is that it is unclear what approach you chose to solve the "directory ends up holding too many files". One could simply discard new traces to do so, one could concatenate to existing files to avoid creating new files, one could even cycle the directory (i.e. path/to/log may become path/to/log.old.1 and path/to/log is recreated as an empty directory when it gets a new file).

    trace2: discard new traces when a target directory has too many files

or something would convey the problem you are solving (i.e. "too many files" implying negative performance and usability impact coming from it) and solution (i.e. "discard new traces"), if it is the approach you have chosen.

Show 14 quoted lines

> +	/* check sentinel */
> +	strbuf_addbuf(&sentinel_path, &path);
> +	strbuf_addstr(&sentinel_path, OVERLOAD_SENTINEL_NAME);
> +	if (!stat(sentinel_path.buf, &statbuf)) {
> +		ret = 1;
> +		goto cleanup;
> +	}
> +
> +	/* check file count */
> +	dirp = opendir(path.buf);
> +	while (file_count < tr2env_max_files && dirp && readdir(dirp))
> +		file_count++;
> +	if (dirp)
> +		closedir(dirp);

So, until we accumulate too many files in the directory, every process when it starts tracing will scan the output directory. Hopefully the max is not set to too large a value.