git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: git@vger.kernel.org
Cc: Josh Steadmon <steadmon@google.com>, Jeff King <peff@peff.net>,
	Jeff Hostetler <jeffhost@microsoft.com>
Subject: RFC: error codes on exit
Date: Wed, 19 May 2021 16:34:24 -0700	[thread overview]
Message-ID: <YKWggLGDhTOY+lcy@google.com> (raw)

Hi,

(Danger, jrn is wading into error handling again...)

At $DAYJOB we are setting up some alerting for some bot fleets and
developer workstations, using trace2 as the data source.  Having
trace2 has been great --- combined with gradual weekly rollouts of
"next", it helps us to understand quickly when a change is creating a
regression for users, which hopefully improves the quality of Git for
everyone.

One kind of signal we haven't been able to make good use of is error
rates.  The problem is that a die() call can be an indication of

 a. the user asked to do something that isn't sensible, and we kindly
    rebuked the user

 b. we contacted a server, and the server was not happy with our
    request

 c. the local Git repository is corrupt

 d. we ran out of resources (e.g., disk space)

 e. we encountered an internal error in handling the user's
    legitimate request

and these different cases do not all motivate the same response.
(E.g., if (c) affects just a single bot but produces a high error rate
from that bot, we shouldn't be alarmed; if (d) is happening on a bot,
then we should look into giving it more disk; if (e) is increasing
significantly during a rollout then we should roll back quickly.)

In order to do this, I would like to annotate "exit" events with a
classification of the error.  I'm not too opinionated about what that
classification looks like (bikeshedding welcome!) --- e.g., something
like the enumeration at
https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto
is likely to work fine.

(I'm particularly fond of how that maps to HTTP statuses.  See also
https://github.com/abseil/abseil-cpp/blob/HEAD/absl/status/status.h
for an example of using that kind of enumeration within a single
process.)

The API could look something like

	--- a/cache.h
	+++ b/cache.h
	@@ -590,6 +590,15 @@ int is_git_directory(const char *path);
	  */
	 int is_nonbare_repository_dir(struct strbuf *path);

	+enum git_error_code {
	+	/*
	+	 * Not an error (= HTTP 200)
	+	 */
	+	OK = 0,
	+};
	+NORETURN void fatal(enum git_error_code code, const char *err, ...)
	+	__attribute__((format (printf, 2, 3)));
	+
	 #define READ_GITFILE_ERR_STAT_FAILED 1
	 #define READ_GITFILE_ERR_NOT_A_FILE 2
	 #define READ_GITFILE_ERR_OPEN_FAILED 3

(with new error codes added when they first get used) and a typical
caller could look like

	Subject: xsize_t: tag "cannot handle files this big" as a failed precondition

	Unlike retriable errors, failed preconditions indicate that some
	aspect of the state needs to be changed in order to recover.  Mark
	this error as such to make signals from monitoring in controlled
	environments (e.g., bot fleets or corporate installations of Git)
	easier to understand.

	Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
[...]
	+       /*
	+        * The system is not in a state required for the operation to succeed.
	+        * For example, a file on disk is larger than we can handle.
	+        * (= HTTP 400)
	+        */
	+       FAILED_PRECONDITION = 9,
[...]
	 static inline size_t xsize_t(off_t len)
	 {
		if (len < 0 || len > SIZE_MAX)
	-               die("Cannot handle files this big");
	+               fatal(FAILED_PRECONDITION, "Cannot handle files this big");

Further down the line I can imagine making use of git_error_code
elsewhere for e.g. some limited retries of the corresponding
transaction when we fail to lock a file.

Thoughts?  Good idea?  Bad idea?

Thanks,
Jonathan

             reply	other threads:[~2021-05-19 23:34 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-19 23:34 Jonathan Nieder [this message]
2021-05-20  0:40 ` RFC: error codes on exit Felipe Contreras
2021-05-21 16:53   ` Alex Henrie
2021-05-21 23:20     ` H. Peter Anvin
2021-05-22  4:06       ` Bagas Sanjaya
2021-05-22  8:49       ` Junio C Hamano
2021-05-22  9:08         ` H. Peter Anvin
2021-05-22 21:22         ` Felipe Contreras
2021-05-22 21:29           ` H. Peter Anvin
2021-05-22 21:53             ` Felipe Contreras
2021-05-22 23:02               ` H. Peter Anvin
2021-05-22  9:12     ` Philip Oakley
2021-05-22 21:19       ` Felipe Contreras
2021-05-25 17:24         ` Alex Henrie
2021-05-25 18:43           ` Felipe Contreras
2021-05-20  0:49 ` Junio C Hamano
2021-05-20  1:19   ` Felipe Contreras
2021-05-20  1:55   ` Jonathan Nieder
2021-05-20  2:28     ` Junio C Hamano
2021-05-20 13:28 ` Jeff King
2021-05-20 17:47   ` Jonathan Nieder
2021-05-21  9:43     ` Jeff King
2021-05-20 15:09 ` Jeff Hostetler
2021-05-21  1:33   ` brian m. carlson
2021-05-21  1:20 ` brian m. carlson
2021-05-26  8:21 ` Ævar Arnfjörð Bjarmason

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=YKWggLGDhTOY+lcy@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jeffhost@microsoft.com \
    --cc=peff@peff.net \
    --cc=steadmon@google.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public 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).