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

Re: [PATCH v2 10/10] bundle-uri: test missing bundles with heuristic

From
Victoria Dye <vdye@github.com>
Date
Jan 27, 2023, 19:21 UTC
Message-ID
<b3f2992d-f614-5cc7-f606-d3607a154685@github.com>
In-Reply-To
<676522615ad0e8f24099ef35a0f39367e5f688ae.1674487310.git.gitgitgadget@gmail.com>
Derrick Stolee via GitGitGadget wrote:
Show 21 quoted lines
> From: Derrick Stolee <derrickstolee@github.com>
> 
> The creationToken heuristic uses a different mechanism for downloading
> bundles from the "standard" approach. Specifically: it uses a concrete
> order based on the creationToken values and attempts to download as few
> bundles as possible. It also modifies local config to store a value for
> future fetches to avoid downloading bundles, if possible.
> 
> However, if any of the individual bundles has a failed download, then
> the logic for the ordering comes into question. It is important to avoid
> infinite loops, assigning invalid creation token values in config, but
> also to be opportunistic as possible when downloading as many bundles as
> seem appropriate.
> 
> These tests were used to inform the implementation of
> fetch_bundles_by_token() in bundle-uri.c, but are being added
> independently here to allow focusing on faulty downloads. There may be
> more cases that could be added that result in modifications to
> fetch_bundles_by_token() as interesting data shapes reveal themselves in
> real scenarios.
> 
The expanded testing is great, thanks for adding it!
Show 50 quoted lines
> +	# Case 2: middle bundle does not exist, only two bundles can unbundle
> +	cat >"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF &&
> +	[bundle]
> +		version = 1
> +		mode = all
> +		heuristic = creationToken
> +
> +	[bundle "bundle-1"]
> +		uri = bundle-1.bundle
> +		creationToken = 1
> +
> +	[bundle "bundle-2"]
> +		uri = fake.bundle
> +		creationToken = 2
> +
> +	[bundle "bundle-3"]
> +		uri = bundle-3.bundle
> +		creationToken = 3
> +
> +	[bundle "bundle-4"]
> +		uri = bundle-4.bundle
> +		creationToken = 4
> +	EOF
> +
> +	GIT_TRACE2_EVENT="$(pwd)/trace-clone-2.txt" \
> +	git clone --single-branch --branch=base \
> +		--bundle-uri="$HTTPD_URL/bundle-list" \
> +		"$HTTPD_URL/smart/fetch.git" download-2 &&
> +
> +	# Bundle failure does not set these configs.
> +	test_must_fail git -C download-2 config fetch.bundleuri &&
> +	test_must_fail git -C download-2 config fetch.bundlecreationtoken &&
> +
> +	cat >expect <<-EOF &&
> +	$HTTPD_URL/bundle-list
> +	$HTTPD_URL/bundle-4.bundle
> +	$HTTPD_URL/bundle-3.bundle
> +	$HTTPD_URL/fake.bundle
> +	$HTTPD_URL/bundle-1.bundle
> +	EOF
> +	test_remote_https_urls <trace-clone-2.txt >actual &&
> +	test_cmp expect actual &&
> +
> +	# Only base bundle unbundled.
> +	git -C download-2 for-each-ref --format="%(refname)" "refs/bundles/*" >refs &&
> +	cat >expect <<-EOF &&
> +	refs/bundles/base
> +	refs/bundles/right
> +	EOF
> +	test_cmp expect refs &&

Maybe I'm misreading, but I don't think the comment ("Only base bundle unbundled") lines up with the expected bundle refs (both bundle-1 ('refs/bundles/base') and bundle-3 ('refs/bundles/right') seem to be unbundled).

Show 53 quoted lines
> +
> +	# Case 3: top bundle does not exist, rest unbundle fine.
> +	cat >"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF &&
> +	[bundle]
> +		version = 1
> +		mode = all
> +		heuristic = creationToken
> +
> +	[bundle "bundle-1"]
> +		uri = bundle-1.bundle
> +		creationToken = 1
> +
> +	[bundle "bundle-2"]
> +		uri = bundle-2.bundle
> +		creationToken = 2
> +
> +	[bundle "bundle-3"]
> +		uri = bundle-3.bundle
> +		creationToken = 3
> +
> +	[bundle "bundle-4"]
> +		uri = fake.bundle
> +		creationToken = 4
> +	EOF
> +
> +	GIT_TRACE2_EVENT="$(pwd)/trace-clone-3.txt" \
> +	git clone --single-branch --branch=base \
> +		--bundle-uri="$HTTPD_URL/bundle-list" \
> +		"$HTTPD_URL/smart/fetch.git" download-3 &&
> +
> +	# As long as we have continguous successful downloads,
> +	# we _do_ set these configs.
> +	test_cmp_config -C download-3 "$HTTPD_URL/bundle-list" fetch.bundleuri &&
> +	test_cmp_config -C download-3 3 fetch.bundlecreationtoken &&
> +
> +	cat >expect <<-EOF &&
> +	$HTTPD_URL/bundle-list
> +	$HTTPD_URL/fake.bundle
> +	$HTTPD_URL/bundle-3.bundle
> +	$HTTPD_URL/bundle-2.bundle
> +	$HTTPD_URL/bundle-1.bundle
> +	EOF
> +	test_remote_https_urls <trace-clone-3.txt >actual &&
> +	test_cmp expect actual &&
> +
> +	# All bundles failed to unbundle
> +	git -C download-3 for-each-ref --format="%(refname)" "refs/bundles/*" >refs &&
> +	cat >expect <<-EOF &&
> +	refs/bundles/base
> +	refs/bundles/left
> +	refs/bundles/right
> +	EOF
> +	test_cmp expect refs

Similar issue with the comment here - it says that all bundles *failed* to unbundle, but the test case description ("Case 3: top bundle does not exist, rest unbundle fine.") and the result show bundle-1, bundle-2, and bundle-3 all unbundling successfully.

Show 22 quoted lines
> +'
> +
> +# Expand the bundle list to include other interesting shapes, specifically
> +# interesting for use when fetching from a previous state.
> +#
> +# ---------------- bundle-7
> +#       7
> +#     _/|\_
> +# ---/--|--\------ bundle-6
> +#   5   |   6
> +# --|---|---|----- bundle-4
> +#   |   4   |
> +#   |  / \  /
> +# --|-|---|/------ bundle-3 (the client will be caught up to this point.)
> +#   \ |   3
> +# ---\|---|------- bundle-2
> +#     2   |
> +# ----|---|------- bundle-1
> +#      \ /
> +#       1
> +#       |
> +# (previous commits)
...
> +	# Case 1: all bundles exist: successful unbundling of all bundles
...
> +	# Case 2: middle bundle does not exist, only bundle-4 can unbundle
...
> +	# Case 3: top bundle does not exist, rest unbundle fine.

The rest of these cases look okay and, at a high-level, it's helpful to have these additional tests covering a different topology.

Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee
Message 58 of 79 in “Bundle URIs V: creationToken heuristic for incremental fetches”
  1. 0/8 Bundle URIs V: creationToken heuristic for incremental fetchesDerrick Stolee via GitGitGadget, Jan 6, 2023
  2. 2/8 bundle-uri: parse bundle.heuristic=creationTokenDerrick Stolee via GitGitGadget, Jan 6, 2023
  3. Junio C HamanoJan 9, 2023
  4. Derrick StoleeJan 9, 2023
  5. Victoria DyeJan 17, 2023
  6. 1/8 t5558: add tests for creationToken heuristicDerrick Stolee via GitGitGadget, Jan 6, 2023
  7. Victoria DyeJan 17, 2023
  8. Derrick StoleeJan 17, 2023
  9. 6/8 bundle-uri: drop bundle.flag from design docDerrick Stolee via GitGitGadget, Jan 6, 2023
  10. Victoria DyeJan 19, 2023
  11. 5/8 clone: set fetch.bundleURI if appropriateDerrick Stolee via GitGitGadget, Jan 6, 2023
  12. Victoria DyeJan 19, 2023
  13. Derrick StoleeJan 20, 2023
  14. 7/8 fetch: fetch from an external bundle URIDerrick Stolee via GitGitGadget, Jan 6, 2023
  15. Victoria DyeJan 19, 2023
  16. Derrick StoleeJan 20, 2023
  17. 4/8 bundle-uri: download in creationToken orderDerrick Stolee via GitGitGadget, Jan 6, 2023
  18. Junio C HamanoJan 9, 2023
  19. Derrick StoleeJan 9, 2023
  20. Victoria DyeJan 19, 2023
  21. Derrick StoleeJan 20, 2023
  22. 8/8 bundle-uri: store fetch.bundleCreationTokenDerrick Stolee via GitGitGadget, Jan 6, 2023
  23. Victoria DyeJan 19, 2023
  24. Derrick StoleeJan 20, 2023
  25. 3/8 bundle-uri: parse bundle.<id>.creationToken valuesDerrick Stolee via GitGitGadget, Jan 6, 2023
  26. Junio C HamanoJan 9, 2023
  27. Derrick StoleeJan 9, 2023
  28. Victoria DyeJan 17, 2023
  29. 00/10 Bundle URIs V: creationToken heuristic for incremental fetchesDerrick Stolee via GitGitGadget, Jan 23, 2023
  30. 01/10 bundle: optionally skip reachability walkDerrick Stolee via GitGitGadget, Jan 23, 2023
  31. Junio C HamanoJan 23, 2023
  32. Derrick StoleeJan 23, 2023
  33. Junio C HamanoJan 23, 2023
  34. Junio C HamanoJan 23, 2023
  35. Derrick StoleeJan 24, 2023
  36. 01/11 bundle: test unbundling with incomplete historyDerrick Stolee, Jan 24, 2023
  37. Junio C HamanoJan 24, 2023
  38. 02/11 bundle: verify using connected()Derrick Stolee, Jan 24, 2023
  39. Junio C HamanoJan 24, 2023
  40. Derrick StoleeJan 24, 2023
  41. Junio C HamanoJan 24, 2023
  42. Junio C HamanoJan 24, 2023
  43. Junio C HamanoJan 23, 2023
  44. 02/10 t5558: add tests for creationToken heuristicDerrick Stolee via GitGitGadget, Jan 23, 2023
  45. Victoria DyeJan 27, 2023
  46. 04/10 bundle-uri: parse bundle.<id>.creationToken valuesDerrick Stolee via GitGitGadget, Jan 23, 2023
  47. 03/10 bundle-uri: parse bundle.heuristic=creationTokenDerrick Stolee via GitGitGadget, Jan 23, 2023
  48. 05/10 bundle-uri: download in creationToken orderDerrick Stolee via GitGitGadget, Jan 23, 2023
  49. Victoria DyeJan 27, 2023
  50. Junio C HamanoJan 27, 2023
  51. Derrick StoleeJan 30, 2023
  52. Junio C HamanoJan 30, 2023
  53. Derrick StoleeJan 30, 2023
  54. 06/10 clone: set fetch.bundleURI if appropriateDerrick Stolee via GitGitGadget, Jan 23, 2023
  55. 07/10 bundle-uri: drop bundle.flag from design docDerrick Stolee via GitGitGadget, Jan 23, 2023
  56. 09/10 bundle-uri: store fetch.bundleCreationTokenDerrick Stolee via GitGitGadget, Jan 23, 2023
  57. 10/10 bundle-uri: test missing bundles with heuristicDerrick Stolee via GitGitGadget, Jan 23, 2023
  58. Victoria DyeJan 27, 2023
  59. Derrick StoleeJan 30, 2023
  60. 08/10 fetch: fetch from an external bundle URIDerrick Stolee via GitGitGadget, Jan 23, 2023
  61. Victoria DyeJan 27, 2023
  62. Victoria DyeJan 27, 2023
  63. 00/11 Bundle URIs V: creationToken heuristic for incremental fetchesDerrick Stolee via GitGitGadget, Jan 31, 2023
  64. 01/11 bundle: test unbundling with incomplete historyDerrick Stolee via GitGitGadget, Jan 31, 2023
  65. 02/11 bundle: verify using check_connected()Derrick Stolee via GitGitGadget, Jan 31, 2023
  66. Junio C HamanoJan 31, 2023
  67. Derrick StoleeJan 31, 2023
  68. Junio C HamanoJan 31, 2023
  69. 03/11 t5558: add tests for creationToken heuristicDerrick Stolee via GitGitGadget, Jan 31, 2023
  70. 04/11 bundle-uri: parse bundle.heuristic=creationTokenDerrick Stolee via GitGitGadget, Jan 31, 2023
  71. 05/11 bundle-uri: parse bundle.<id>.creationToken valuesDerrick Stolee via GitGitGadget, Jan 31, 2023
  72. Junio C HamanoJan 31, 2023
  73. 07/11 clone: set fetch.bundleURI if appropriateDerrick Stolee via GitGitGadget, Jan 31, 2023
  74. 06/11 bundle-uri: download in creationToken orderDerrick Stolee via GitGitGadget, Jan 31, 2023
  75. 08/11 bundle-uri: drop bundle.flag from design docDerrick Stolee via GitGitGadget, Jan 31, 2023
  76. 09/11 fetch: fetch from an external bundle URIDerrick Stolee via GitGitGadget, Jan 31, 2023
  77. 11/11 bundle-uri: test missing bundles with heuristicDerrick Stolee via GitGitGadget, Jan 31, 2023
  78. 10/11 bundle-uri: store fetch.bundleCreationTokenDerrick Stolee via GitGitGadget, Jan 31, 2023
  79. Junio C HamanoJan 31, 2023

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.