Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gix fetch sometimes gets stuck during negociation #882

Closed
1 task done
g2p opened this issue Jun 7, 2023 · 4 comments
Closed
1 task done

gix fetch sometimes gets stuck during negociation #882

g2p opened this issue Jun 7, 2023 · 4 comments
Assignees

Comments

@g2p
Copy link

g2p commented Jun 7, 2023

Duplicates

  • I have searched the existing issues

Current behavior 😯

Fetching the attached repo with gix (gitoxide 0.26.0) doesn't converge, gets stuck on negotiate (round 3) 1 steps [ ===

Followup to #861 and #812.

fedora-kickstarts.git.tar.gz

Expected behavior 🤔

Fetch the repo

Steps to reproduce 🕹

untar, cd
gix fetch
Running git fetch once will prevent the issue with gix fetch, don't run git before reproducing

@Byron Byron self-assigned this Jun 7, 2023
@Byron
Copy link
Owner

Byron commented Jun 7, 2023

Thanks for giving it a try and for providing a reproduction :)!

I am indeed able to reproduce the issue and know what the problem is. In short, despite having faithfully implemented negotiation, I skimped on one details of V1 negotiation which will deadlock unless one adds what considerable complication to the implementation. Even though I was able to satisfy my own tests, it's not enough to satisfy this example.

However, I am sure I will be back with a fix shortly and hope that one will work better in the real world :D.

Byron added a commit that referenced this issue Jun 7, 2023
… specialty (#882).

Seeing READY means a pack will follow, which is exactly the kind of knowledge we want.
Thus we now either listen to the client OR to what the server says.
@Byron
Copy link
Owner

Byron commented Jun 7, 2023

PR #884 has the fix, and I will release it shortly, but while investigating I noticed something else: git is able to do the negotiation in one round with the default algorithm, whereas gix needs two.

00b4want dc9e3b2ceab166a538c0466c126a8b1111d3fa1c multi_ack_detailed no-done side-band-64k thin-pack include-tag ofs-delta deepen-since deepen-not agent=git/2.39.2.(Apple.Git-143)
0032want 69c726f7536cde6acf36697f7ba112ad3107e7ac
0032want c3b160775a3b159f949ba931dcb68e520a460e12
00000032have efcff0a4c2e5f381dbf5ad5928471c64469d2eb1
0032have bb780fa1bbf7f03fc5ae3b4cfce6703e08913aa2
0032have e351fc2680638a703f93084cf2f8d8c85c36b847
0032have cad6175774df336670f6b894d71f00519cdec903
0032have c0297b29390dbebe99e7b5fac609eb7c10585552
0032have 17178da61a5c4f6bf8480b5139e070138f635977
0032have 4697b2de60d5a52a94e8bba7c3b4ce1e021efa48
0032have 6fbeb2efb05ed01a3a6bd73e04f45cc50ffae488
0032have 879a7d74092f9d324d9488f981cab625f557d6b4
0032have 0b2fe8612b6e4fc7814c269a26dcabaa4d2a798e
0032have 022f04580e1261dd8bfb13c9d9173d65eddd5c45
0032have 3cfe995016eebde3204dab81b30d9768aab0df17
0032have f76a16b585fec6f9b9d534e676a732b76d81585f
0032have 13aac55e296d9f0538e40e873a66f611b56f3a55
0032have e9eade1e56a7061aa1ad4a49943ba9dce3d495e0
0032have ceb046a0ca17fe0a2572bfff26b9f17f30d2b545
0000

---- Server Reply

0038ACK efcff0a4c2e5f381dbf5ad5928471c64469d2eb1 common
0038ACK bb780fa1bbf7f03fc5ae3b4cfce6703e08913aa2 common
0038ACK e351fc2680638a703f93084cf2f8d8c85c36b847 common
0038ACK cad6175774df336670f6b894d71f00519cdec903 common
0038ACK c0297b29390dbebe99e7b5fac609eb7c10585552 common
0038ACK 17178da61a5c4f6bf8480b5139e070138f635977 common
0038ACK 4697b2de60d5a52a94e8bba7c3b4ce1e021efa48 common
0038ACK 6fbeb2efb05ed01a3a6bd73e04f45cc50ffae488 common
0038ACK 879a7d74092f9d324d9488f981cab625f557d6b4 common
0038ACK 0b2fe8612b6e4fc7814c269a26dcabaa4d2a798e common
0038ACK 022f04580e1261dd8bfb13c9d9173d65eddd5c45 common
0038ACK 3cfe995016eebde3204dab81b30d9768aab0df17 common
0038ACK f76a16b585fec6f9b9d534e676a732b76d81585f common
0038ACK 13aac55e296d9f0538e40e873a66f611b56f3a55 common
0038ACK e9eade1e56a7061aa1ad4a49943ba9dce3d495e0 common
0038ACK ceb046a0ca17fe0a2572bfff26b9f17f30d2b545 common
0037ACK ceb046a0ca17fe0a2572bfff26b9f17f30d2b545 ready
0008NAK
0031ACK ceb046a0ca17fe0a2572bfff26b9f17f30d2b545

gix clearly starts with somewhat different tips, which probably is the reason that it accidentally doesn't see the commits that it should see to help the server along more quickly. This is unexpected as the underlying algorithm was supposed to be the same, but maybe the reference iteration order is different (even though it shouldn't as it is sorted). For now I am not fretting it, and thought it was interesting to mention that the skipping algorithm provided by gix also manages to converge within one round.

❯ cargo run -- -r /Users/byron/Downloads/fedora-kickstarts.git -c http.proxy=localhost:9090 -c fetch.negotiationAlgorithm=skipping fetch
    Finished dev [unoptimized + debuginfo] target(s) in 0.50s
     Running `target/debug/gix -r /Users/byron/Downloads/fedora-kickstarts.git -c 'http.proxy=localhost:9090' -c fetch.negotiationAlgorithm=skipping fetch`
 14:38:00 read pack done 9.6KB in 0.00s (3.1MB/s)
 14:38:00  indexing done 15.0 objects in 0.00s (15.3k objects/s)
 14:38:00 decompressing done 18.1KB in 0.00s (18.3MB/s)
 14:38:00     Resolving done 15.0 objects in 0.06s (270.0 objects/s)
 14:38:00      Decoding done 34.4KB in 0.06s (620.0KB/s)
 14:38:00 writing index file done 1.5KB in 0.00s (5.9MB/s)
 14:38:00  create index file done 15.0 objects in 0.06s (262.0 objects/s)
+refs/heads/*:refs/remotes/origin/*
        17178da61a5c4f6bf8480b5139e070138f635977 refs/heads/arm-minimal-xkbcommon -> refs/remotes/origin/arm-minimal-xkbcommon [up-to-date]
        c0297b29390dbebe99e7b5fac609eb7c10585552 refs/heads/arm-minimal-xkbcommon-38 -> refs/remotes/origin/arm-minimal-xkbcommon-38 [up-to-date]
        ec43a9d960b8a9b4e363664a168bbaaba1f935ca refs/heads/drop-retired-games -> refs/remotes/origin/drop-retired-games [up-to-date]
        59e73c16ee88d29d0c0d923bba43c0cdc53d8a01 refs/heads/f21 -> refs/remotes/origin/f21 [up-to-date]
        b5bfeaf8f34f8a31fa0520fe10f4ad3ba77fb002 refs/heads/f22 -> refs/remotes/origin/f22 [up-to-date]
        dfde53409893754a749e390bb63b8b110211ceac refs/heads/f23 -> refs/remotes/origin/f23 [up-to-date]
        a550d625485cf6953ff169b4df6f426783fbd09c refs/heads/f24 -> refs/remotes/origin/f24 [up-to-date]
        26be0f2d850f2107204f6d1c19577276e78eb85d refs/heads/f25 -> refs/remotes/origin/f25 [up-to-date]
        3c02bf385d5f4deb94e77ebbc9526e676c214502 refs/heads/f26 -> refs/remotes/origin/f26 [up-to-date]
        aaec0f81291bd7191c08c5a245f168e4c2489f65 refs/heads/f27 -> refs/remotes/origin/f27 [up-to-date]
        b3e9d978c76664ced99ac92369770eeb59497f5f refs/heads/f28 -> refs/remotes/origin/f28 [up-to-date]
        fcf1418f1936468aa39c9d20b22ab976badd4eac refs/heads/f29 -> refs/remotes/origin/f29 [up-to-date]
        fae67ed38400fe8e6706cf760c7fcae8704df8e3 refs/heads/f30 -> refs/remotes/origin/f30 [up-to-date]
        b4e313947d38b85621bdb7c8b970688014f23c1b refs/heads/f31 -> refs/remotes/origin/f31 [up-to-date]
        13aac55e296d9f0538e40e873a66f611b56f3a55 refs/heads/f32 -> refs/remotes/origin/f32 [up-to-date]
        e9eade1e56a7061aa1ad4a49943ba9dce3d495e0 refs/heads/f33 -> refs/remotes/origin/f33 [up-to-date]
        022f04580e1261dd8bfb13c9d9173d65eddd5c45 refs/heads/f34 -> refs/remotes/origin/f34 [up-to-date]
        3cfe995016eebde3204dab81b30d9768aab0df17 refs/heads/f35 -> refs/remotes/origin/f35 [up-to-date]
        4697b2de60d5a52a94e8bba7c3b4ce1e021efa48 refs/heads/f36 -> refs/remotes/origin/f36 [up-to-date]
        dc9e3b2ceab166a538c0466c126a8b1111d3fa1c refs/heads/f37 -> refs/remotes/origin/f37 [fast-forward]
        69c726f7536cde6acf36697f7ba112ad3107e7ac refs/heads/f38 -> refs/remotes/origin/f38 [fast-forward]
        c3b160775a3b159f949ba931dcb68e520a460e12 refs/heads/main -> refs/remotes/origin/main [fast-forward]
        879a7d74092f9d324d9488f981cab625f557d6b4 refs/heads/minimization-cleanups -> refs/remotes/origin/minimization-cleanups [up-to-date]
refs/tags/*:refs/tags/* (implicit, due to auto-tag)
        skipped 80 tags known to the remote without bearing on this commit-graph. Use `gix remote ref-map` to list them.
pack  file: "/Users/byron/Downloads/fedora-kickstarts.git/objects/pack/pack-c289c128dac825fb9d496a6e4040fbb1ca5f3738.pack"
index file: "/Users/byron/Downloads/fedora-kickstarts.git/objects/pack/pack-c289c128dac825fb9d496a6e4040fbb1ca5f3738.idx"
server sent 805 tips, 702 were filtered due to 1 refspec(s).

Fortunately, no matter what, the pack that gix and git receive, no matter which algorithm, always contain 15 objects so the result is still the same.

Byron added a commit that referenced this issue Jun 7, 2023
… specialty (#882).

Seeing READY means a pack will follow, which is exactly the kind of knowledge we want.
Thus we now either listen to the client OR to what the server says.

Note that this remedy relies on `multi-ack-detailed`, which some very old servers might
not support. For those we would be out-of-luck, but that seems acceptable.
@Byron
Copy link
Owner

Byron commented Jun 7, 2023

Could you build locally and retry, now that the fix was released via gix-protocol?

I should be able to create a new gix CLI release later today as well (depends on #883) - this will probably wait until there is an actual change to gitoxide-core, it's hard to release if nothing actually changed.

@g2p
Copy link
Author

g2p commented Jun 8, 2023

This fix works for me, thank you!

@g2p g2p closed this as completed Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants