{"thread":{"id":"61806","subject":"[PATCH 0/2] Fix background maintenance regression in Git 2.45.0","startedAt":"2024-07-18T19:55:50Z","lastAt":"2024-07-19T16:20:41Z","messageCount":12,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano","Taylor Blau","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"498936","messageId":"pull.1764.git.1721332546.gitgitgadget@gmail.com","threadId":"61806","inReplyTo":null,"subject":"[PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-18T19:55:44Z","receivedAt":"2024-07-18T19:55:50Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Here is an issue I noticed while exploring issues with my local copy of a\nlarge monorepo. I was intending to show some engineers how nice the objects\nwere maintained by background maintenance, but saw hundreds of small\npack-files that were up to two months old. This time matched when I upgraded\nto the microsoft/git fork that included the 2.45.0 release of Git.\n\nThe issue is that 'git multi-pack-index repack' was taught to call 'git\npack-objects' with the new '--stdin-packs' option. However, this changes the\nobject selection algorithm. Instead of using the objects referenced by the\nmulti-pack-index, it compares pack-files using a list of \"included\" and\n\"excluded\" pack-files. This loses some granularity of how the\nmulti-pack-index chooses among duplicate objects.\n\nThe end result is that some objects that would normally have been included\nin the new pack-file are no longer included. The copy that the\nmulti-pack-index references is in the pack-file that was intended to be\nrepacked, so that pack-file cannot be expired in the next 'git\nmulti-pack-index expire' step and is included again in the batch of objects\nto repack.\n\nIn the context of the change that is reverted by this series, it seems the\nmotivation of the change was two-fold:\n\n 1. some I/O benefits to using pack names over object names, and\n 2. the ability to use an object walk to improve delta compression.\n\nIn my local prototyping, I've found that we could improve 'git pack-objects'\nto use an object walk when given a set of objects over stdin without needing\nto use pack-file names. I do not believe the '--stdin-packs' option should\nbe used for the 'git multi-pack-index repack' mechanism, or at least should\nbe done with great care and only in specific cases where some assumptions\ncan be made around duplicate objects and closure under reachability.\n\nHowever, the prototype I've built to get these benefits is non-trivial due\nto working to guarantee that partial clones do not accidentally download\nmissing blobs. That will follow in a separate series that can be reviewed at\na slower pace.\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n t5319: add failing test case for repack/expire\n midx-write: revert use of --stdin-packs\n\n midx-write.c | 18 ++++++------\n t/t5319-multi-pack-index.sh | 55 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 64 insertions(+), 9 deletions(-)\n\n\nbase-commit: bea9ecd24b0c3bf06cab4a851694fe09e7e51408\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1764%2Fderrickstolee%2Fincremental-repack-fix-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1764/derrickstolee/incremental-repack-fix-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1764\n-- \ngitgitgadget\n"},{"id":"498937","messageId":"9b8e2012c9107f99e19c541113ae6a405e38a92f.1721332546.git.gitgitgadget@gmail.com","threadId":"61806","inReplyTo":"pull.1764.git.1721332546.gitgitgadget@gmail.com","subject":"[PATCH 1/2] t5319: add failing test case for repack/expire","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-18T19:55:45Z","receivedAt":"2024-07-18T19:55:51Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nGit 2.45.0 included the change b7d6f23a171 (midx-write.c: use\n`--stdin-packs` when repacking, 2024-04-01) which caused the 'git\nmulti-pack-index repack' command to use 'git pack-objects --stdin-packs'\ninstead of listing the objects to repack. While this change was\nmotivated by efficient cross-process communication and the ability to\nimprove delta compression, it breaks a fundamental function of the\n'incremental-repack' task that is enabled by default in Scalar clones or\nGit repositories that run 'git maintenance start'.\n\nThe 'incremental-repack' task performs a two-step process of the\n'expire' and 'repack' subcommands of the 'git multi-pack-index' builtin.\nThe 'expire' command removes any pack-files listed in the\nmulti-pack-index but without any referenced objects. The 'repack' task\nthen finds a batch of pack-files to repack and sends their objects to\n'git pack-objects'. Both the pack-files chosen for the batch and the\nobjects chosen to repack are based on the ones that the multi-pack-index\nreferences. Objects that appear in a pack-file but have a duplicate copy\nin a newer pack-file are not considered in this case. Since the\nmulti-pack-index references only the newest copy of an object, this\nallows the next 'incremental-repack' task to remove the pack-files in\nthe next 'expire' task. This delay is intentional due to how Windows\nhandles may block deletion of files with open read handles.\n\nHowever, the mentioned commit changed this behavior to divorce the set\nof objects referenced by the multi-pack-index and instead use a set of\n\"included\" and \"excluded\" pack-files in the 'git pack-objects' builtin.\nWhen a pack-file is selected as \"included\", only the objects it contains\nbut are not in any \"excluded\" pack-files are considered for repacking.\nThis has led to client repositories failing to remove old pack-files as\nthey still have some referenced objects. This grows over time until the\npoint that Git is trying to repack the same pack-files over and over.\n\nFor now, create a test case that demonstrates the expected behavior, but\nalso fails in its final line. The setup here it attempting to recreate a\ntypical situation for a repository that uses a blobless partial clone.\nThere would be a large initial pack-file from the clone that is never\nselected in the 'repack' batch. There are other pack-files that have a\ncombination of new objects from incremental fetches and possibly blobs\nthat are not connected to those incremental fetches; these blobs could\nbe filled in from commands like 'git checkout' or 'git blame'. The\npack-files also have some overlap on purpose so test-1 has some\nduplicates in test-2 and test-2 has some duplicates in test-3.\n\nAt the end of the test, the test-2 pack-file still exists though it\nshould have been expired. This test will pass when reverting the\noffending commit.\n\nSigned-off-by: Derrick Stolee \n---\n t/t5319-multi-pack-index.sh | 55 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 55 insertions(+)\n\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex dd09134db03..327376233c5 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -1004,6 +1004,61 @@ test_expect_success 'repack --batch-size= repacks everything' '\n \t)\n '\n \n+test_expect_failure 'repack/expire loop' '\n+\tgit init repack-expire &&\n+\ttest_when_finished \"rm -fr repack-expire\" &&\n+\t(\n+\t\tcd repack-expire &&\n+\n+\t\ttest_commit_bulk 5 &&\n+\n+\t\t# Create three overlapping pack-files\n+\t\tgit rev-list --objects HEAD~3 >in-1 &&\n+\t\tgit rev-list --objects HEAD~4..HEAD~2 >in-2 &&\n+\t\tgit rev-list --objects HEAD~3..HEAD >in-3 &&\n+\n+\t\t# Create disconnected blobs\n+\t\tobj1=$(git hash-object -w in-1) &&\n+\t\tobj2=$(git hash-object -w in-2) &&\n+\t\tobj3=$(git hash-object -w in-3) &&\n+\n+\t\techo $obj2 >>in-2 &&\n+\t\techo $obj3 >>in-3 &&\n+\n+\t\tfor i in $(test_seq 3)\n+\t\tdo\n+\t\t\tgit pack-objects .git/objects/pack/test-$i \n\nThis reverts b7d6f23a171 (midx-write.c: use `--stdin-packs` when\nrepacking, 2024-04-01) and then marks the test created in the previous\nchange as passing.\n\nThe fundamental issue with the reverted change is that the focus on\npack-files separates the object selection from how the multi-pack-index\nselects a single pack-file for an object ID with multiple copies among\nthe tracked pack-files.\n\nThe change was made with the intention of improving delta compression in\nthe resulting pack-file, but that can be resolved with the existing\nobject list mechanism. There are other potential pitfalls of doing an\nobject walk at this time if the repository is a blobless partial clone,\nand that will require additional testing on top of the one that changes\nhere.\n\nSigned-off-by: Derrick Stolee \n---\n midx-write.c | 18 +++++++++---------\n t/t5319-multi-pack-index.sh | 2 +-\n 2 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/midx-write.c b/midx-write.c\nindex 65e69d2de78..960cc46250e 100644\n--- a/midx-write.c\n+++ b/midx-write.c\n@@ -1474,8 +1474,7 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n \trepo_config_get_bool(r, \"repack.usedeltabaseoffset\", &delta_base_offset);\n \trepo_config_get_bool(r, \"repack.usedeltaislands\", &use_delta_islands);\n \n-\tstrvec_pushl(&cmd.args, \"pack-objects\", \"--stdin-packs\", \"--non-empty\",\n-\t\t NULL);\n+\tstrvec_push(&cmd.args, \"pack-objects\");\n \n \tstrvec_pushf(&cmd.args, \"%s/pack/pack\", object_dir);\n \n@@ -1499,15 +1498,16 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n \t}\n \n \tcmd_in = xfdopen(cmd.in, \"w\");\n-\tfor (i = 0; i < m->num_packs; i++) {\n-\t\tstruct packed_git *p = m->packs[i];\n-\t\tif (!p)\n+\n+\tfor (i = 0; i < m->num_objects; i++) {\n+\t\tstruct object_id oid;\n+\t\tuint32_t pack_int_id = nth_midxed_pack_int_id(m, i);\n+\n+\t\tif (!include_pack[pack_int_id])\n \t\t\tcontinue;\n \n-\t\tif (include_pack[i])\n-\t\t\tfprintf(cmd_in, \"%s\\n\", pack_basename(p));\n-\t\telse\n-\t\t\tfprintf(cmd_in, \"^%s\\n\", pack_basename(p));\n+\t\tnth_midxed_object_oid(&oid, m, i);\n+\t\tfprintf(cmd_in, \"%s\\n\", oid_to_hex(&oid));\n \t}\n \tfclose(cmd_in);\n \ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex 327376233c5..5cce0be19e6 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -1004,7 +1004,7 @@ test_expect_success 'repack --batch-size= repacks everything' '\n \t)\n '\n \n-test_expect_failure 'repack/expire loop' '\n+test_expect_success 'repack/expire loop' '\n \tgit init repack-expire &&\n \ttest_when_finished \"rm -fr repack-expire\" &&\n \t(\n-- \ngitgitgadget\n"},{"id":"498945","messageId":"xmqqle1ynz18.fsf@gitster.g","threadId":"61806","inReplyTo":"pull.1764.git.1721332546.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-18T21:57:55Z","receivedAt":"2024-07-18T21:58:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> Here is an issue I noticed while exploring issues with my local copy of a\n> large monorepo. I was intending to show some engineers how nice the objects\n> were maintained by background maintenance, but saw hundreds of small\n> pack-files that were up to two months old. This time matched when I upgraded\n> to the microsoft/git fork that included the 2.45.0 release of Git.\n\nI almost said \"wow, perfect timing on the -rc1 day\", but then\nrealized that this is not a regression during _this_ cycle, but a\ncycle ago.\n\nYou already Cc'ed Taylor, whom I would have asked for help if I were\nthe one who found this issue, which is good.\n\nWill queue.\n\nThanks.\n"},{"id":"498947","messageId":"ZpmZVAL5MihrNVbN@nand.local","threadId":"61806","inReplyTo":"xmqqle1ynz18.fsf@gitster.g","subject":"Re: [PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-18T22:38:12Z","receivedAt":"2024-07-18T22:38:18Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Jul 18, 2024 at 02:57:55PM -0700, Junio C Hamano wrote:\n> You already Cc'ed Taylor, whom I would have asked for help if I were\n> the one who found this issue, which is good.\n\nIt looks like there is a small typo in my email address as\n\"me@ttayllorr.com\" instead of \"me@ttaylorr.com\", but I saw the message\nanyway ;-).\n\nThanks,\nTaylor\n"},{"id":"498949","messageId":"ZpmcK23coi5Qqm7E@nand.local","threadId":"61806","inReplyTo":"pull.1764.git.1721332546.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-18T22:50:19Z","receivedAt":"2024-07-18T22:50:22Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Jul 18, 2024 at 07:55:44PM +0000, Derrick Stolee via GitGitGadget wrote:\n> The issue is that 'git multi-pack-index repack' was taught to call 'git\n> pack-objects' with the new '--stdin-packs' option. However, this changes the\n> object selection algorithm. Instead of using the objects referenced by the\n> multi-pack-index, it compares pack-files using a list of \"included\" and\n> \"excluded\" pack-files. This loses some granularity of how the\n> multi-pack-index chooses among duplicate objects.\n\nThank you for looking at this so carefully! Let me double check my own\nunderstanding.\n\nSuppose we have a MIDX with some pack 'P' and say, some commit object\n'C' which appears in that pack. Let's also suppose we have another pack\n'Q' in the same MIDX which also contains 'C', but the MIDX selected its\ncopy from pack 'P'.\n\nIf we want to combine 'P' with some other packs (excluding 'Q'), then\nthe input to --stdin-packs will look something like:\n\n P.pack\n ^Q.pack\n ...\n\nAnd the resulting pack would not contain 'C', since we would reject it\nvia: add_object_entry_from_pack() -> want_object_in_pack() ->\nwant_found_object() -> has_object_kept_pack(). The final function there\nwould find a copy of 'C' in 'Q', and since 'Q' is excluded, we would\nreject 'C' as unwanted.\n\nSo the resulting pack would not contain 'C', and the MIDX would hold\nonto its copy from pack 'P', resulting in 'P' being both (a) in the set\nof packs to repack together, but also (b) non-expireable, since it has\nat least one object selected from it in the MIDX.\n\n> The end result is that some objects that would normally have been included\n> in the new pack-file are no longer included. The copy that the\n> multi-pack-index references is in the pack-file that was intended to be\n> repacked, so that pack-file cannot be expired in the next 'git\n> multi-pack-index expire' step and is included again in the batch of objects\n> to repack.\n\nI think this matches my own understanding, but let me know if I'm\nmissing something. Assuming I'm thinking about this the same way you\nare, the fix (stop using --stdin-packss) makes sense to me.\n\nThanks,\nTaylor\n"},{"id":"498960","messageId":"118b164e-67c5-4dfc-b440-62b8986bf356@gmail.com","threadId":"61806","inReplyTo":"ZpmcK23coi5Qqm7E@nand.local","subject":"Re: [PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2024-07-19T13:21:51Z","receivedAt":"2024-07-19T13:21:53Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/18/24 6:50 PM, Taylor Blau wrote:\n> On Thu, Jul 18, 2024 at 07:55:44PM +0000, Derrick Stolee via GitGitGadget wrote:\n>> The issue is that 'git multi-pack-index repack' was taught to call 'git\n>> pack-objects' with the new '--stdin-packs' option. However, this changes the\n>> object selection algorithm. Instead of using the objects referenced by the\n>> multi-pack-index, it compares pack-files using a list of \"included\" and\n>> \"excluded\" pack-files. This loses some granularity of how the\n>> multi-pack-index chooses among duplicate objects.\n> \n> Thank you for looking at this so carefully! Let me double check my own\n> understanding.\n> \n> Suppose we have a MIDX with some pack 'P' and say, some commit object\n> 'C' which appears in that pack. Let's also suppose we have another pack\n> 'Q' in the same MIDX which also contains 'C', but the MIDX selected its\n> copy from pack 'P'.\n> \n> If we want to combine 'P' with some other packs (excluding 'Q'), then\n> the input to --stdin-packs will look something like:\n> \n> P.pack\n> ^Q.pack\n> ...\n> \n> And the resulting pack would not contain 'C', since we would reject it\n> via: add_object_entry_from_pack() -> want_object_in_pack() ->\n> want_found_object() -> has_object_kept_pack(). The final function there\n> would find a copy of 'C' in 'Q', and since 'Q' is excluded, we would\n> reject 'C' as unwanted.\n> \n> So the resulting pack would not contain 'C', and the MIDX would hold\n> onto its copy from pack 'P', resulting in 'P' being both (a) in the set\n> of packs to repack together, but also (b) non-expireable, since it has\n> at least one object selected from it in the MIDX.\n> \n>> The end result is that some objects that would normally have been included\n>> in the new pack-file are no longer included. The copy that the\n>> multi-pack-index references is in the pack-file that was intended to be\n>> repacked, so that pack-file cannot be expired in the next 'git\n>> multi-pack-index expire' step and is included again in the batch of objects\n>> to repack.\n> \n> I think this matches my own understanding, but let me know if I'm\n> missing something. Assuming I'm thinking about this the same way you\n> are, the fix (stop using --stdin-packss) makes sense to me.\n\nYour interpretation matches mine. Thanks for the careful read.\n\nI think we can accomplish similar goals that match the reasoning for\n--stdin-packs (better deltas while also limiting the object walk to the \nrepacked objects) with some changes to read_object_list_from_stdin(),\nbut that's a more subtle kind of change.\n\nTaking this change as-is will cause a regression in the quality of\ndelta choices, but recovers our ability to expire \"repacked\" pack-files\nin all cases.\n\nThanks,\n-Stolee\n"},{"id":"498961","messageId":"4b154e61-a9ca-4c46-8548-b072d65a64b9@gmail.com","threadId":"61806","inReplyTo":"ZpmZVAL5MihrNVbN@nand.local","subject":"Re: [PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2024-07-19T13:23:06Z","receivedAt":"2024-07-19T13:23:08Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/18/24 6:38 PM, Taylor Blau wrote:\n> On Thu, Jul 18, 2024 at 02:57:55PM -0700, Junio C Hamano wrote:\n>> You already Cc'ed Taylor, whom I would have asked for help if I were\n>> the one who found this issue, which is good.\n> \n> It looks like there is a small typo in my email address as\n> \"me@ttayllorr.com\" instead of \"me@ttaylorr.com\", but I saw the message\n> anyway ;-).\n\nWhoops! Sorry about that. Repeated too many letters. I've fixed the\nGitGitGadget cover letter in case a v2 is required.\n\nThanks,\n-Stolee\n"},{"id":"498962","messageId":"81c7bd02-0c2d-452f-800e-ca0d3853a941@gmail.com","threadId":"61806","inReplyTo":"xmqqle1ynz18.fsf@gitster.g","subject":"Re: [PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2024-07-19T13:24:55Z","receivedAt":"2024-07-19T13:24:57Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/18/24 5:57 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> Here is an issue I noticed while exploring issues with my local copy of a\n>> large monorepo. I was intending to show some engineers how nice the objects\n>> were maintained by background maintenance, but saw hundreds of small\n>> pack-files that were up to two months old. This time matched when I upgraded\n>> to the microsoft/git fork that included the 2.45.0 release of Git.\n> \n> I almost said \"wow, perfect timing on the -rc1 day\", but then\n> realized that this is not a regression during _this_ cycle, but a\n> cycle ago.\n\nI almost waited until after the release, but I wanted to put the\ninformation out there just in case you were interested in taking it\ninto 2.46.0 or were planning on a 2.45.3.\n\nThanks,\n-Stolee\n"},{"id":"498963","messageId":"ZppsWZ6u05U65Blp@nand.local","threadId":"61806","inReplyTo":"118b164e-67c5-4dfc-b440-62b8986bf356@gmail.com","subject":"Re: [PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-07-19T13:38:33Z","receivedAt":"2024-07-19T13:38:35Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Jul 19, 2024 at 09:21:51AM -0400, Derrick Stolee wrote:\n> On 7/18/24 6:50 PM, Taylor Blau wrote:\n>\n> > I think this matches my own understanding, but let me know if I'm\n> > missing something. Assuming I'm thinking about this the same way you\n> > are, the fix (stop using --stdin-packss) makes sense to me.\n>\n> Your interpretation matches mine. Thanks for the careful read.\n>\n> I think we can accomplish similar goals that match the reasoning for\n> --stdin-packs (better deltas while also limiting the object walk to the\n> repacked objects) with some changes to read_object_list_from_stdin(),\n> but that's a more subtle kind of change.\n\nFWIW, the main motivation for that change was to limit the amount of\ncross-process I/O that was necessary to generate the new pack. I figured\nthat for relatively small amounts of packs which contain relatively\nlarge amounts of objects that it would be more efficient to write out\nthe pack names than the object names.\n\nI was thinking a little bit about how we would alter the behavior of\n'--stdin-packs' to match what the 'multi-pack-index repack' caller\nneeds. I agree that it is possible, and I doubly agree that it is subtle\n;-).\n\nTBH, I think that the amount of I/O we're potentially saving is dwarfed\nby the amount of I/O and CPU time it takes to actually generate the new\npack, so I doubt the effort to make such a subtle change would be all\nthat worthwhile, though certainly an interesting exercise ;-).\n\n> Taking this change as-is will cause a regression in the quality of\n> delta choices, but recovers our ability to expire \"repacked\" pack-files\n> in all cases.\n\nYeah, definitely.\n\nThanks,\nTaylor\n"},{"id":"498968","messageId":"xmqq34o5l8j7.fsf@gitster.g","threadId":"61806","inReplyTo":"81c7bd02-0c2d-452f-800e-ca0d3853a941@gmail.com","subject":"Re: [PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-19T15:13:16Z","receivedAt":"2024-07-19T15:13:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 7/18/24 5:57 PM, Junio C Hamano wrote:\n>> \"Derrick Stolee via GitGitGadget\" writes:\n>> \n>>> Here is an issue I noticed while exploring issues with my local copy of a\n>>> large monorepo. I was intending to show some engineers how nice the objects\n>>> were maintained by background maintenance, but saw hundreds of small\n>>> pack-files that were up to two months old. This time matched when I upgraded\n>>> to the microsoft/git fork that included the 2.45.0 release of Git.\n>> I almost said \"wow, perfect timing on the -rc1 day\", but then\n>> realized that this is not a regression during _this_ cycle, but a\n>> cycle ago.\n>\n> I almost waited until after the release, but I wanted to put the\n> information out there just in case you were interested in taking it\n> into 2.46.0 or were planning on a 2.45.3.\n\nYup, thanks but this is not exactly a repository breaking data\ncorruption bug, and did not look ultra urgent. Especially if we\nwant to pursue a solution that helps both expiring stale packs\nbetter (which is what you are restoring) and making better delta\nchain selection (which may be what you are losing) at the same time,\nsuch a change could become a source of data corruption bug, so I'd\nprefer to see it started early in a cycle, rather as a last-minute\n\"let's fix this too\".\n\nThanks.\n"},{"id":"498971","messageId":"861420aa-e423-4475-8eba-8ec5187c52d4@gmail.com","threadId":"61806","inReplyTo":"xmqq34o5l8j7.fsf@gitster.g","subject":"Re: [PATCH 0/2] Fix background maintenance regression in Git 2.45.0","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2024-07-19T16:20:38Z","receivedAt":"2024-07-19T16:20:41Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/19/24 11:13 AM, Junio C Hamano wrote:\n> Derrick Stolee writes:\n> \n>> On 7/18/24 5:57 PM, Junio C Hamano wrote:\n>>> \"Derrick Stolee via GitGitGadget\" writes:\n>>>\n>>>> Here is an issue I noticed while exploring issues with my local copy of a\n>>>> large monorepo. I was intending to show some engineers how nice the objects\n>>>> were maintained by background maintenance, but saw hundreds of small\n>>>> pack-files that were up to two months old. This time matched when I upgraded\n>>>> to the microsoft/git fork that included the 2.45.0 release of Git.\n>>> I almost said \"wow, perfect timing on the -rc1 day\", but then\n>>> realized that this is not a regression during _this_ cycle, but a\n>>> cycle ago.\n>>\n>> I almost waited until after the release, but I wanted to put the\n>> information out there just in case you were interested in taking it\n>> into 2.46.0 or were planning on a 2.45.3.\n> \n> Yup, thanks but this is not exactly a repository breaking data\n> corruption bug, and did not look ultra urgent. Especially if we\n> want to pursue a solution that helps both expiring stale packs\n> better (which is what you are restoring) and making better delta\n> chain selection (which may be what you are losing) at the same time,\n> such a change could become a source of data corruption bug, so I'd\n> prefer to see it started early in a cycle, rather as a last-minute\n> \"let's fix this too\".\n\nI agree with this assessment. The microsoft/git fork has taken the\nrevert as users of that fork have a higher chance of being affected.\n\nThanks,\n-Stolee\n"}]}