git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3] fetch-pack: write effective filter to trace2

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Jul 26, 2022, 16:27 UTC
Message-ID
<20220726162712.1774355-1-jonathantanmy@google.com>
In-Reply-To
<20220715172943.2681492-1-jonathantanmy@google.com>

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(-)
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
Previous: Jonathan Tan
Message 15 of 15 in “fetch-pack: write effective filter to trace2”
  1. fetch-pack: write effective filter to trace2Jonathan Tan, Jul 15, 2022
  2. Jeff HostetlerJul 15, 2022
  3. Junio C HamanoJul 15, 2022
  4. Jonathan TanJul 15, 2022
  5. Junio C HamanoJul 15, 2022
  6. Jonathan TanJul 15, 2022
  7. Jeff HostetlerJul 18, 2022
  8. Junio C HamanoJul 18, 2022
  9. Jonathan TanJul 18, 2022
  10. Junio C HamanoJul 18, 2022
  11. fetch-pack: write effective filter to trace2Jonathan Tan, Jul 18, 2022
  12. Junio C HamanoJul 18, 2022
  13. Junio C HamanoJul 25, 2022
  14. Jonathan TanJul 26, 2022
  15. fetch-pack: write effective filter to trace2Jonathan Tan, Jul 26, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.