[PATCH v3] fetch-pack: write effective filter to trace2
Administrators of a managed Git environment (like the one at $DAYJOB) might want to quantify the performance change of fetches with and without filters from the client's point of view, and also detect if a server does not support it. Therefore, log the filter information being sent to the server whenever a fetch (or clone) occurs. Note that this is not necessarily the same as what's specified on the CLI, because during a fetch, the configured filter is used whenever a filter is not specified on the CLI.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Here's v3, with an updated function name.
---
Range-diff against v2:
1: d3ea7d3b95 ! 1: 17cef49fcc fetch-pack: write effective filter to trace2
@@ Commit message
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
- Thanks everyone for your comments. I've renamed the keys and
- distinguished the case in which the server does not support filters
- following Jeff's suggestions.
-
- A few small discussion points:
- - whether we need the print_verbose now that we have traces
- - should we log if no filter is specified
- - if yes, what key should it be logged under (I used "filter/none" for
- now)
+ Here's v3, with an updated function name.
## fetch-pack.c ##
@@ fetch-pack.c: static void mark_tips(struct fetch_negotiator *negotiator,
return;
}
-+static void write_and_trace_filter(struct fetch_pack_args *args,
-+ struct strbuf *req_buf,
-+ int server_supports_filter)
++static void send_filter(struct fetch_pack_args *args,
++ struct strbuf *req_buf,
++ int server_supports_filter)
+{
+ if (args->filter_options.choice) {
+ const char *spec =
@@ fetch-pack.c: static int find_common(struct fetch_negotiator *negotiator,
- expand_list_objects_filter_spec(&args->filter_options);
- packet_buf_write(&req_buf, "filter %s", spec);
- }
-+ write_and_trace_filter(args, &req_buf, server_supports_filtering);
++ send_filter(args, &req_buf, server_supports_filtering);
packet_buf_flush(&req_buf);
state_len = req_buf.len;
@@ fetch-pack.c: static int send_fetch_request(struct fetch_negotiator *negotiator,
- } else if (args->filter_options.choice) {
- warning("filtering not recognized by server, ignoring");
- }
-+ write_and_trace_filter(args, &req_buf,
-+ server_supports_feature("fetch", "filter", 0));
++ send_filter(args, &req_buf,
++ server_supports_feature("fetch", "filter", 0));
if (server_supports_feature("fetch", "packfile-uris", 0)) {
int i; fetch-pack.c | 40 ++++++++++++++++++++++++++--------------
1 file changed, 26 insertions(+), 14 deletions(-)
Show changes to fetch-pack.c +26 −14
diff --git a/fetch-pack.c b/fetch-pack.c
index cb6647d657..a44eb32a71 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -292,6 +292,29 @@ static void mark_tips(struct fetch_negotiator *negotiator,
return;
}
+static void send_filter(struct fetch_pack_args *args,
+ struct strbuf *req_buf,
+ int server_supports_filter)
+{
+ if (args->filter_options.choice) {
+ const char *spec =
+ expand_list_objects_filter_spec(&args->filter_options);
+ if (server_supports_filter) {
+ print_verbose(args, _("Server supports filter"));
+ packet_buf_write(req_buf, "filter %s", spec);
+ trace2_data_string("fetch", the_repository,
+ "filter/effective", spec);
+ } else {
+ warning("filtering not recognized by server, ignoring");
+ trace2_data_string("fetch", the_repository,
+ "filter/unsupported", spec);
+ }
+ } else {
+ trace2_data_string("fetch", the_repository,
+ "filter/none", "");
+ }
+}
+
static int find_common(struct fetch_negotiator *negotiator,
struct fetch_pack_args *args,
int fd[2], struct object_id *result_oid,
@@ -389,11 +412,7 @@ static int find_common(struct fetch_negotiator *negotiator,
packet_buf_write(&req_buf, "deepen-not %s", s->string);
}
}
- if (server_supports_filtering && args->filter_options.choice) {
- const char *spec =
- expand_list_objects_filter_spec(&args->filter_options);
- packet_buf_write(&req_buf, "filter %s", spec);
- }
+ send_filter(args, &req_buf, server_supports_filtering);
packet_buf_flush(&req_buf);
state_len = req_buf.len;
@@ -1323,15 +1342,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
die(_("Server does not support shallow requests"));
/* Add filter */
- if (server_supports_feature("fetch", "filter", 0) &&
- args->filter_options.choice) {
- const char *spec =
- expand_list_objects_filter_spec(&args->filter_options);
- print_verbose(args, _("Server supports filter"));
- packet_buf_write(&req_buf, "filter %s", spec);
- } else if (args->filter_options.choice) {
- warning("filtering not recognized by server, ignoring");
- }
+ send_filter(args, &req_buf,
+ server_supports_feature("fetch", "filter", 0));
if (server_supports_feature("fetch", "packfile-uris", 0)) {
int i;
--
2.37.1.359.gd136c6c3e2-goog
