@matheusbsbs / matheus-bagliano / commits / 4fa3f00abb

Fork 0

fetch-pack: in protocol v2, in_vain only after ACK

When fetching, Git stops negotiation when it has sent at least MAX_IN_VAIN (which is 256) "have" lines without having any of them ACK-ed. But this is supposed to trigger only after the first ACK, as pack-protocol.txt says: However, the 256 limit *only* turns on in the canonical client implementation if we have received at least one "ACK %s continue" during a prior round. This helps to ensure that at least one common ancestor is found before we give up entirely. The code path for protocol v0 observes this, but not protocol v2, resulting in shorter negotiation rounds but significantly larger packfiles. Teach the code path for protocol v2 to check this criterion only after at least one ACK was received. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Apr 27, 2020 at 17:01 UTC 4fa3f00abb55c3334cce71e201a5ff2c70f8561f

2 files changed +27 -4

fetch-pack.c

+9 -4

@@ -1143,6 +1143,7 @@ static void add_common(struct strbuf *req_buf, struct oidset *common)

1143 }

1144

1145 static int add_haves(struct fetch_negotiator *negotiator,

1146 + int seen_ack,

1147 struct strbuf *req_buf,

1148 int *haves_to_send, int *in_vain)

1149 {

@@ -1157,7 +1158,7 @@ static int add_haves(struct fetch_negotiator *negotiator,

1158 }

1159

1160 *in_vain += haves_added;

1160 - if (!haves_added || *in_vain >= MAX_IN_VAIN) {

1161 + if (!haves_added || (seen_ack && *in_vain >= MAX_IN_VAIN)) {

1162 /* Send Done */

1163 packet_buf_write(req_buf, "done\n");

1164 ret = 1;

@@ -1173,7 +1174,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,

1174 struct fetch_pack_args *args,

1175 const struct ref *wants, struct oidset *common,

1176 int *haves_to_send, int *in_vain,

1176 - int sideband_all)

1177 + int sideband_all, int seen_ack)

1178 {

1179 int ret = 0;

1180 struct strbuf req_buf = STRBUF_INIT;

@@ -1230,7 +1231,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,

1231 add_common(&req_buf, common);

1232

1233 /* Add initial haves */

1233 - ret = add_haves(negotiator, &req_buf, haves_to_send, in_vain);

1234 + ret = add_haves(negotiator, seen_ack, &req_buf,

1235 + haves_to_send, in_vain);

1236 }

1237

1238 /* Send request */

@@ -1464,6 +1466,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

1466 int haves_to_send = INITIAL_FLUSH;

1467 struct fetch_negotiator negotiator_alloc;

1468 struct fetch_negotiator *negotiator;

1469 + int seen_ack = 0;

1470

1471 if (args->no_dependents) {

1472 negotiator = NULL;

@@ -1520,7 +1523,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

1523 if (send_fetch_request(negotiator, fd[1], args, ref,

1524 &common,

1525 &haves_to_send, &in_vain,

1523 - reader.use_sideband))

1526 + reader.use_sideband,

1527 + seen_ack))

1528 state = FETCH_GET_PACK;

1529 else

1530 state = FETCH_PROCESS_ACKS;

@@ -1533,6 +1537,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

1537 break;

1538 case COMMON_FOUND:

1539 in_vain = 0;

1540 + seen_ack = 1;

1541 /* fallthrough */

1542 case NO_COMMON_FOUND:

1543 state = FETCH_SEND_REQUEST;

t/t5500-fetch-pack.sh

+18

@@ -385,6 +385,24 @@ test_expect_success 'clone shallow with packed refs' '

385 test_cmp count8.expected count8.actual

386 '

387

388 +test_expect_success 'in_vain not triggered before first ACK' '

389 + rm -rf myserver myclient trace &&

390 + git init myserver &&

391 + test_commit -C myserver foo &&

392 + git clone "file://$(pwd)/myserver" myclient &&

393 +

394 + # MAX_IN_VAIN is 256. Because of batching, the client will send 496

395 + # (16+32+64+128+256) commits, not 256, before giving up. So create 496

396 + # irrelevant commits.

397 + test_commit_bulk -C myclient 496 &&

398 +

399 + # The new commit that the client wants to fetch.

400 + test_commit -C myserver bar &&

401 +

402 + GIT_TRACE_PACKET="$(pwd)/trace" git -C myclient fetch --progress origin &&

403 + test_i18ngrep "Total 3 " trace

404 +'

405 +

406 test_expect_success 'fetch in shallow repo unreachable shallow objects' '

407 (

408 git clone --bare --branch B --single-branch "file://$(pwd)/." no-reflog &&