Maintenance II: prefetch, loose-objects, incremental-repack tasks #696
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
derrickstolee
changed the title
|
/submit |
|
Submitted as pull.696.git.1596731424.gitgitgadget@gmail.com |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -29,6 +29,8 @@ | |||
| #include "tree.h" | |||
There was a problem hiding this comment.
On the Git mailing list, Son Luong Ngoc wrote (reply to this):
Hi Derrick,
> On Aug 6, 2020, at 18:30, Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> When repacking during the 'incremental-repack' task, we use the
> --batch-size option in 'git multi-pack-index repack'. The initial setting
> used --batch-size=0 to repack everything into a single pack-file. This is
> not sustainable for a large repository. The amount of work required is
> also likely to use too many system resources for a background job.
>
> Update the 'incremental-repack' task by dynamically computing a
> --batch-size option based on the current pack-file structure.
>
> The dynamic default size is computed with this idea in mind for a client
> repository that was cloned from a very large remote: there is likely one
> "big" pack-file that was created at clone time. Thus, do not try
> repacking it as it is likely packed efficiently by the server.
>
> Instead, we select the second-largest pack-file, and create a batch size
> that is one larger than that pack-file. If there are three or more
> pack-files, then this guarantees that at least two will be combined into
> a new pack-file.
I have been using this strategy with git-care.sh [1] with large success.
However it worth to note that there are still edge case where I observed that
pack count keep increasing because using '--batch-size=<second-biggest-pack>+1'
did not resulted in any repacking.
In one case, I have observed a local copy went up to 160+ packs without being able
to repack.
I have been considering whether a strategy such as falling back to the '(3rd biggest
pack size) + 1' and 4th and 5th and so on... when midx repack call resulted in no-op,
as that was how I fixed my repo when the edge case happen.
Such strategy would require a way to detect midx repack to signal when no-op happen,
so something like 'git multi-pack-index repack --batch-size=123456 --exit-code' would
be much desirable.
>
> Of course, this means that the second-largest pack-file size is likely
> to grow over time and may eventually surpass the initially-cloned
> pack-file. Recall that the pack-file batch is selected in a greedy
> manner: the packs are considered from oldest to newest and are selected
> if they have size smaller than the batch size until the total selected
> size is larger than the batch size. Thus, that oldest "clone" pack will
> be first to repack after the new data creates a pack larger than that.
>
> We also want to place some limits on how large these pack-files become,
> in order to bound the amount of time spent repacking. A maximum
> batch-size of two gigabytes means that large repositories will never be
> packed into a single pack-file using this job, but also that repack is
> rather expensive. This is a trade-off that is valuable to have if the
> maintenance is being run automatically or in the background. Users who
> truly want to optimize for space and performance (and are willing to pay
> the upfront cost of a full repack) can use the 'gc' task to do so.
>
> Create a test for this two gigabyte limit by creating an EXPENSIVE test
> that generates two pack-files of roughly 2.5 gigabytes in size, then
> performs an incremental repack. Check that the --batch-size argument in
> the subcommand uses the hard-coded maximum.
>
> Helped-by: Chris Torek <chris.torek@gmail.com>
> Reported-by: Son Luong Ngoc <sluongng@gmail.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Generally, I have found working with '--batch-size' to be a bit unpredictable.
I wonder if we could tweak the behavior somewhat so that its more consistent
to use and test?
Thanks a lot for making this happen.
Hope this patch would make it in stable soon
Cheers,
Son Luong.
[1]: https://github.com/sluongng/git-care
There was a problem hiding this comment.
On the Git mailing list, Derrick Stolee wrote (reply to this):
On 8/6/2020 1:02 PM, Son Luong Ngoc wrote:
> Hi Derrick,
>
>> On Aug 6, 2020, at 18:30, Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
>>
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> When repacking during the 'incremental-repack' task, we use the
>> --batch-size option in 'git multi-pack-index repack'. The initial setting
>> used --batch-size=0 to repack everything into a single pack-file. This is
>> not sustainable for a large repository. The amount of work required is
>> also likely to use too many system resources for a background job.
>>
>> Update the 'incremental-repack' task by dynamically computing a
>> --batch-size option based on the current pack-file structure.
>>
>> The dynamic default size is computed with this idea in mind for a client
>> repository that was cloned from a very large remote: there is likely one
>> "big" pack-file that was created at clone time. Thus, do not try
>> repacking it as it is likely packed efficiently by the server.
>>
>> Instead, we select the second-largest pack-file, and create a batch size
>> that is one larger than that pack-file. If there are three or more
>> pack-files, then this guarantees that at least two will be combined into
>> a new pack-file.
>
> I have been using this strategy with git-care.sh [1] with large success.
> However it worth to note that there are still edge case where I observed that
> pack count keep increasing because using '--batch-size=<second-biggest-pack>+1'
> did not resulted in any repacking.
> In one case, I have observed a local copy went up to 160+ packs without being able
> to repack.
>
> I have been considering whether a strategy such as falling back to the '(3rd biggest
> pack size) + 1' and 4th and 5th and so on... when midx repack call resulted in no-op,
> as that was how I fixed my repo when the edge case happen.
>
> Such strategy would require a way to detect midx repack to signal when no-op happen,
> so something like 'git multi-pack-index repack --batch-size=123456 --exit-code' would
> be much desirable.
>
>>
>> Of course, this means that the second-largest pack-file size is likely
>> to grow over time and may eventually surpass the initially-cloned
>> pack-file. Recall that the pack-file batch is selected in a greedy
>> manner: the packs are considered from oldest to newest and are selected
>> if they have size smaller than the batch size until the total selected
>> size is larger than the batch size. Thus, that oldest "clone" pack will
>> be first to repack after the new data creates a pack larger than that.
>>
>> We also want to place some limits on how large these pack-files become,
>> in order to bound the amount of time spent repacking. A maximum
>> batch-size of two gigabytes means that large repositories will never be
>> packed into a single pack-file using this job, but also that repack is
>> rather expensive. This is a trade-off that is valuable to have if the
>> maintenance is being run automatically or in the background. Users who
>> truly want to optimize for space and performance (and are willing to pay
>> the upfront cost of a full repack) can use the 'gc' task to do so.
>>
>> Create a test for this two gigabyte limit by creating an EXPENSIVE test
>> that generates two pack-files of roughly 2.5 gigabytes in size, then
>> performs an incremental repack. Check that the --batch-size argument in
>> the subcommand uses the hard-coded maximum.
>>
>> Helped-by: Chris Torek <chris.torek@gmail.com>
>> Reported-by: Son Luong Ngoc <sluongng@gmail.com>
>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>
> Generally, I have found working with '--batch-size' to be a bit unpredictable.
> I wonder if we could tweak the behavior somewhat so that its more consistent
> to use and test?
Thanks for continuing to test with this model. My experience has been limited
to the --batch-size=2g option on repos that typically have >15gb of commit
and tree data in their pack directory.
One bit of unpredictability that we've seen is that the --batch-size uses
the "expected size" of a pack-file to select if it should be repacked. This
only tracks the objects that are referenced in that pack, so when we have
new packs that were "un-thinned" by duplicating delta-bases, those affect
the expected size of a repack.
Here is a great reason to have this series be split. While part I stabilizes,
we can take the time to re-evaluate this strategy. It might require updating
the multi-pack-index builtin itself.
One thing to think about is to focus on the different possible sizes of a
repository. If the entire pack-directory is small, then we might as well
repack everything. (What should the limit be? 2gb? configurable?) In the
case of a larger directory, should we just use the --batch-size logic with
that limit, or should we create a new option for which packs to repack?
For example, the recent simulation [1] of downloading fetch packs and
running this maintenance did see extra space that could be recovered with
the current logic. However, what if we could specify "repack everything
except the largest pack" exactly? That would do exactly what this task
is _intending_, as long as that resulting pack-file does not exceed the
2gb maximum by too much.
[1] https://lore.kernel.org/git/d50fbb33-9be3-1c48-2277-8bf894df734f@gmail.com/
I will think more on this. I'm open to alternate strategies.
Thanks,
-Stolee
|
This patch series was integrated into seen via git@24f74cc. |
|
This branch is now known as |
|
This patch series was integrated into seen via git@4512e12. |
|
This patch series was integrated into seen via git@a7d7de8. |
|
This patch series was integrated into seen via git@b79973c. |
|
This patch series was integrated into seen via git@2d7fb38. |
|
This patch series was integrated into seen via git@259f64d. |
|
This patch series was integrated into seen via git@4e23072. |
|
This patch series was integrated into seen via git@1d8642a. |
|
This patch series was integrated into seen via git@29d14d1. |
derrickstolee
force-pushed
the
maintenance/gc
branch
from
407c123 to
a78dbc7
Compare
|
This patch series was integrated into seen via git@d382ef5. |
|
This patch series was integrated into seen via git@1e955ae. |
|
This patch series was integrated into seen via git@8e26d96. |
|
This patch series was integrated into seen via git@cc806c3. |
derrickstolee
force-pushed
the
maintenance/gc
branch
from
a78dbc7 to
d9d8607
Compare
derrickstolee
force-pushed
the
stolee/maintenance/builtin
branch
from
a5d1914 to
8f862c1
Compare
|
This patch series was integrated into seen via git@13828d2. |
|
This patch series was integrated into seen via git@14a81c8. |
derrickstolee
force-pushed
the
maintenance/gc
branch
from
d9d8607 to
8ff86f0
Compare
|
This patch series was integrated into seen via git@680b2ec. |
|
This patch series was integrated into seen via git@2fe046b. |
|
User |
When working with very large repositories, an incremental 'git fetch' command can download a large amount of data. If there are many other users pushing to a common repo, then this data can rival the initial pack-file size of a 'git clone' of a medium-size repo. Users may want to keep the data on their local repos as close as possible to the data on the remote repos by fetching periodically in the background. This can break up a large daily fetch into several smaller hourly fetches. The task is called "prefetch" because it is work done in advance of a foreground fetch to make that 'git fetch' command much faster. However, if we simply ran 'git fetch <remote>' in the background, then the user running a foreground 'git fetch <remote>' would lose some important feedback when a new branch appears or an existing branch updates. This is especially true if a remote branch is force-updated and this isn't noticed by the user because it occurred in the background. Further, the functionality of 'git push --force-with-lease' becomes suspect. When running 'git fetch <remote> <options>' in the background, use the following options for careful updating: 1. --no-tags prevents getting a new tag when a user wants to see the new tags appear in their foreground fetches. 2. --refmap= removes the configured refspec which usually updates refs/remotes/<remote>/* with the refs advertised by the remote. While this looks confusing, this was documented and tested by b40a502 (fetch: document and test --refmap="", 2020-01-21), including this sentence in the documentation: Providing an empty `<refspec>` to the `--refmap` option causes Git to ignore the configured refspecs and rely entirely on the refspecs supplied as command-line arguments. 3. By adding a new refspec "+refs/heads/*:refs/prefetch/<remote>/*" we can ensure that we actually load the new values somewhere in our refspace while not updating refs/heads or refs/remotes. By storing these refs here, the commit-graph job will update the commit-graph with the commits from these hidden refs. 4. --prune will delete the refs/prefetch/<remote> refs that no longer appear on the remote. 5. --no-write-fetch-head prevents updating FETCH_HEAD. We've been using this step as a critical background job in Scalar [1] (and VFS for Git). This solved a pain point that was showing up in user reports: fetching was a pain! Users do not like waiting to download the data that was created while they were away from their machines. After implementing background fetch, the foreground fetch commands sped up significantly because they mostly just update refs and download a small amount of new data. The effect is especially dramatic when paried with --no-show-forced-udpates (through fetch.showForcedUpdates=false). [1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/FetchStep.cs Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
One goal of background maintenance jobs is to allow a user to disable auto-gc (gc.auto=0) but keep their repository in a clean state. Without any cleanup, loose objects will clutter the object database and slow operations. In addition, the loose objects will take up extra space because they are not stored with deltas against similar objects. Create a 'loose-objects' task for the 'git maintenance run' command. This helps clean up loose objects without disrupting concurrent Git commands using the following sequence of events: 1. Run 'git prune-packed' to delete any loose objects that exist in a pack-file. Concurrent commands will prefer the packed version of the object to the loose version. (Of course, there are exceptions for commands that specifically care about the location of an object. These are rare for a user to run on purpose, and we hope a user that has selected background maintenance will not be trying to do foreground maintenance.) 2. Run 'git pack-objects' on a batch of loose objects. These objects are grouped by scanning the loose object directories in lexicographic order until listing all loose objects -or- reaching 50,000 objects. This is more than enough if the loose objects are created only by a user doing normal development. We noticed users with _millions_ of loose objects because VFS for Git downloads blobs on-demand when a file read operation requires populating a virtual file. This step is based on a similar step in Scalar [1] and VFS for Git. [1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/LooseObjectsStep.cs Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
derrickstolee
force-pushed
the
maintenance/gc
branch
from
e9bb32f to
f660dd1
Compare
derrickstolee
changed the base branch from
stolee/maintenance/builtin
to
ds/maintenance-part-1
|
/submit |
|
Submitted as pull.696.v4.git.1601037218.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
| @@ -14,3 +14,12 @@ maintenance.commit-graph.auto:: | |||
| reachable commits that are not in the commit-graph file is at least | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index b3fc7c8670..27565c55a2 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -127,4 +127,26 @@ test_expect_success 'loose-objects task' '
> test_cmp packs-between packs-after
> '
>
> +test_expect_success 'maintenance.loose-objects.auto' '
> + git repack -adk &&
> + GIT_TRACE2_EVENT="$(pwd)/trace-lo1.txt" \
> + git -c maintenance.loose-objects.auto=1 maintenance \
> + run --auto --task=loose-objects 2>/dev/null &&
> + test_subcommand ! git prune-packed --quiet <trace-lo1.txt &&
> + printf data-A | git hash-object -t blob --stdin -w &&
> + GIT_TRACE2_EVENT="$(pwd)/trace-loA" \
> + git -c maintenance.loose-objects.auto=2 \
> + maintenance run --auto --task=loose-objects 2>/dev/null &&
> + test_subcommand ! git prune-packed --quiet <trace-loA &&
> + printf data-B | git hash-object -t blob --stdin -w &&
Is it essential for the purpose of the test somehow that the data
used for the test are all incomplete files that lack the end-of-line
at the end of the file? Use of 'printf' sends such a signal to
confuse readers.
Use of test_write_lines to write a single line may feel overkill,
but it may be less cryptic, as newer parts of testsuite are
encouraged to use it over 'echo' and raw 'printf'.
> + GIT_TRACE2_EVENT="$(pwd)/trace-loB" \
> + git -c maintenance.loose-objects.auto=2 \
> + maintenance run --auto --task=loose-objects 2>/dev/null &&
> + test_subcommand git prune-packed --quiet <trace-loB &&
> + GIT_TRACE2_EVENT="$(pwd)/trace-loC" \
> + git -c maintenance.loose-objects.auto=2 \
> + maintenance run --auto --task=loose-objects 2>/dev/null &&
> + test_subcommand git prune-packed --quiet <trace-loC
> +'
There was a problem hiding this comment.
On the Git mailing list, Derrick Stolee wrote (reply to this):
On 9/25/2020 2:00 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> + printf data-B | git hash-object -t blob --stdin -w &&
>
> Is it essential for the purpose of the test somehow that the data
> used for the test are all incomplete files that lack the end-of-line
> at the end of the file? Use of 'printf' sends such a signal to
> confuse readers.
>
> Use of test_write_lines to write a single line may feel overkill,
> but it may be less cryptic, as newer parts of testsuite are
> encouraged to use it over 'echo' and raw 'printf'.
I suppose it could be more standard. It's not particularly
important what the data is here, so lacking a newline seems
innocuous enough to me. I'm happy to agree with standards
elsewhere to avoid being a bad example.
A simple s/printf/test_write_lines/ appears to do the trick:
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index f259485990..a1bd0029c6 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -147,12 +147,12 @@ test_expect_success 'maintenance.loose-objects.auto' '
git -c maintenance.loose-objects.auto=1 maintenance \
run --auto --task=loose-objects 2>/dev/null &&
test_subcommand ! git prune-packed --quiet <trace-lo1.txt &&
- printf data-A | git hash-object -t blob --stdin -w &&
+ test_write_lines data-A | git hash-object -t blob --stdin -w &&
GIT_TRACE2_EVENT="$(pwd)/trace-loA" \
git -c maintenance.loose-objects.auto=2 \
maintenance run --auto --task=loose-objects 2>/dev/null &&
test_subcommand ! git prune-packed --quiet <trace-loA &&
- printf data-B | git hash-object -t blob --stdin -w &&
+ test_write_lines data-B | git hash-object -t blob --stdin -w &&
GIT_TRACE2_EVENT="$(pwd)/trace-loB" \
git -c maintenance.loose-objects.auto=2 \
maintenance run --auto --task=loose-objects 2>/dev/null &&
Thanks,
-Stolee
|
This patch series was integrated into seen via git@c82131a. |
|
This patch series was integrated into seen via git@0ad0821. |
|
This patch series was integrated into seen via git@81c89f2. |
The loose-objects task deletes loose objects that already exist in a pack-file, then place the remaining loose objects into a new pack-file. If this step runs all the time, then we risk creating pack-files with very few objects with every 'git commit' process. To prevent overwhelming the packs directory with small pack-files, place a minimum number of objects to justify the task. The 'maintenance.loose-objects.auto' config option specifies a minimum number of loose objects to justify the task to run under the '--auto' option. This defaults to 100 loose objects. Setting the value to zero will prevent the step from running under '--auto' while a negative value will force it to run every time. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
The core.multiPackIndex setting has been around since c4d2522 (config: create core.multiPackIndex setting, 2018-07-12), but has been disabled by default. If a user wishes to use the multi-pack-index feature, then they must enable this config and run 'git multi-pack-index write'. The multi-pack-index feature is relatively stable now, so make the config option true by default. For users that do not use a multi-pack-index, the only extra cost will be a file lookup to see if a multi-pack-index file exists (once per process, per object directory). Also, this config option will be referenced by an upcoming "incremental-repack" task in the maintenance builtin, so move the config option into the repository settings struct. Note that if GIT_TEST_MULTI_PACK_INDEX=1, then we want to ignore the config option and treat core.multiPackIndex as enabled. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Now that the multi-pack-index may be written as part of auto maintenance at the end of a command, reduce the progress output when the operations are quick. Use start_delayed_progress() instead of start_progress(). Update t5319-multi-pack-index.sh to use GIT_PROGRESS_DELAY=0 now that the progress indicators are conditional. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
The previous change cleaned up loose objects using the 'loose-objects' that can be run safely in the background. Add a similar job that performs similar cleanups for pack-files. One issue with running 'git repack' is that it is designed to repack all pack-files into a single pack-file. While this is the most space-efficient way to store object data, it is not time or memory efficient. This becomes extremely important if the repo is so large that a user struggles to store two copies of the pack on their disk. Instead, perform an "incremental" repack by collecting a few small pack-files into a new pack-file. The multi-pack-index facilitates this process ever since 'git multi-pack-index expire' was added in 19575c7 (multi-pack-index: implement 'expire' subcommand, 2019-06-10) and 'git multi-pack-index repack' was added in ce1e4a1 (midx: implement midx_repack(), 2019-06-10). The 'incremental-repack' task runs the following steps: 1. 'git multi-pack-index write' creates a multi-pack-index file if one did not exist, and otherwise will update the multi-pack-index with any new pack-files that appeared since the last write. This is particularly relevant with the background fetch job. When the multi-pack-index sees two copies of the same object, it stores the offset data into the newer pack-file. This means that some old pack-files could become "unreferenced" which I will use to mean "a pack-file that is in the pack-file list of the multi-pack-index but none of the objects in the multi-pack-index reference a location inside that pack-file." 2. 'git multi-pack-index expire' deletes any unreferenced pack-files and updaes the multi-pack-index to drop those pack-files from the list. This is safe to do as concurrent Git processes will see the multi-pack-index and not open those packs when looking for object contents. (Similar to the 'loose-objects' job, there are some Git commands that open pack-files regardless of the multi-pack-index, but they are rarely used. Further, a user that self-selects to use background operations would likely refrain from using those commands.) 3. 'git multi-pack-index repack --bacth-size=<size>' collects a set of pack-files that are listed in the multi-pack-index and creates a new pack-file containing the objects whose offsets are listed by the multi-pack-index to be in those objects. The set of pack- files is selected greedily by sorting the pack-files by modified time and adding a pack-file to the set if its "expected size" is smaller than the batch size until the total expected size of the selected pack-files is at least the batch size. The "expected size" is calculated by taking the size of the pack-file divided by the number of objects in the pack-file and multiplied by the number of objects from the multi-pack-index with offset in that pack-file. The expected size approximates how much data from that pack-file will contribute to the resulting pack-file size. The intention is that the resulting pack-file will be close in size to the provided batch size. The next run of the incremental-repack task will delete these repacked pack-files during the 'expire' step. In this version, the batch size is set to "0" which ignores the size restrictions when selecting the pack-files. It instead selects all pack-files and repacks all packed objects into a single pack-file. This will be updated in the next change, but it requires doing some calculations that are better isolated to a separate change. These steps are based on a similar background maintenance step in Scalar (and VFS for Git) [1]. This was incredibly effective for users of the Windows OS repository. After using the same VFS for Git repository for over a year, some users had _thousands_ of pack-files that combined to up to 250 GB of data. We noticed a few users were running into the open file descriptor limits (due in part to a bug in the multi-pack-index fixed by af96fe3 (midx: add packs to packed_git linked list, 2019-04-29). These pack-files were mostly small since they contained the commits and trees that were pushed to the origin in a given hour. The GVFS protocol includes a "prefetch" step that asks for pre-computed pack- files containing commits and trees by timestamp. These pack-files were grouped into "daily" pack-files once a day for up to 30 days. If a user did not request prefetch packs for over 30 days, then they would get the entire history of commits and trees in a new, large pack-file. This led to a large number of pack-files that had poor delta compression. By running this pack-file maintenance step once per day, these repos with thousands of packs spanning 200+ GB dropped to dozens of pack- files spanning 30-50 GB. This was done all without removing objects from the system and using a constant batch size of two gigabytes. Once the work was done to reduce the pack-files to small sizes, the batch size of two gigabytes means that not every run triggers a repack operation, so the following run will not expire a pack-file. This has kept these repos in a "clean" state. [1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/PackfileMaintenanceStep.cs Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
When repacking during the 'incremental-repack' task, we use the --batch-size option in 'git multi-pack-index repack'. The initial setting used --batch-size=0 to repack everything into a single pack-file. This is not sustainable for a large repository. The amount of work required is also likely to use too many system resources for a background job. Update the 'incremental-repack' task by dynamically computing a --batch-size option based on the current pack-file structure. The dynamic default size is computed with this idea in mind for a client repository that was cloned from a very large remote: there is likely one "big" pack-file that was created at clone time. Thus, do not try repacking it as it is likely packed efficiently by the server. Instead, we select the second-largest pack-file, and create a batch size that is one larger than that pack-file. If there are three or more pack-files, then this guarantees that at least two will be combined into a new pack-file. Of course, this means that the second-largest pack-file size is likely to grow over time and may eventually surpass the initially-cloned pack-file. Recall that the pack-file batch is selected in a greedy manner: the packs are considered from oldest to newest and are selected if they have size smaller than the batch size until the total selected size is larger than the batch size. Thus, that oldest "clone" pack will be first to repack after the new data creates a pack larger than that. We also want to place some limits on how large these pack-files become, in order to bound the amount of time spent repacking. A maximum batch-size of two gigabytes means that large repositories will never be packed into a single pack-file using this job, but also that repack is rather expensive. This is a trade-off that is valuable to have if the maintenance is being run automatically or in the background. Users who truly want to optimize for space and performance (and are willing to pay the upfront cost of a full repack) can use the 'gc' task to do so. Create a test for this two gigabyte limit by creating an EXPENSIVE test that generates two pack-files of roughly 2.5 gigabytes in size, then performs an incremental repack. Check that the --batch-size argument in the subcommand uses the hard-coded maximum. Helped-by: Chris Torek <chris.torek@gmail.com> Reported-by: Son Luong Ngoc <sluongng@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
The incremental-repack task updates the multi-pack-index by deleting pack- files that have been replaced with new packs, then repacking a batch of small pack-files into a larger pack-file. This incremental repack is faster than rewriting all object data, but is slower than some other maintenance activities. The 'maintenance.incremental-repack.auto' config option specifies how many pack-files should exist outside of the multi-pack-index before running the step. These pack-files could be created by 'git fetch' commands or by the loose-objects task. The default value is 10. Setting the option to zero disables the task with the '--auto' option, and a negative value makes the task run every time. Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
derrickstolee
force-pushed
the
maintenance/gc
branch
from
f660dd1 to
9fc7c0e
Compare
|
This patch series was integrated into seen via git@8d3fab5. |
|
This patch series was integrated into seen via git@021a762. |
|
This patch series was integrated into next via git@c2a46f3. |
|
This patch series was integrated into seen via git@3b07ac9. |
|
This patch series was integrated into seen via git@2432c00. |
|
This patch series was integrated into seen via git@ec4648c. |
|
This patch series was integrated into seen via git@1dd03d4. |
|
This patch series was integrated into seen via git@b47c9fa. |
|
This patch series was integrated into seen via git@ad8a1e9. |
|
This patch series was integrated into seen via git@6e30e4d. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This series is based on ds/maintenance-part-1 [2].
This patch series contains 9 patches that were going to be part of v4 of ds/maintenance [1], but the discussion has gotten really long. To help, I'm splitting out the portions that create and test the 'maintenance' builtin from the additional tasks (prefetch, loose-objects, incremental-repack) that can be brought in later.
[1] https://lore.kernel.org/git/pull.671.git.1594131695.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/pull.695.v3.git.1598380426.gitgitgadget@gmail.com/
As detailed in [2], the 'git maintenance run' subcommand will run certain tasks based on config options or the --task= arguments. The --auto option indicates to the task to only run based on some internal check that there has been "enough" change in that domain to merit the work. In the case of the 'gc' task, this also reduces the amount of work done.
The new maintenance tasks in this series are:
These tasks are all disabled by default, but can be enabled with config options or run explicitly using "git maintenance run --task=".
Since [2] replaced the 'git gc --auto' calls with 'git maintenance run --auto' at the end of some Git commands, users could replace the 'gc' task with these lighter-weight changes for foreground maintenance.
The 'git maintenance' builtin has a 'run' subcommand so it can be extended later with subcommands that manage background maintenance, such as 'start' or 'stop'. These are not the subject of this series, as it is important to focus on the maintenance activities themselves. I have an RFC series for this available at [3].
[3] https://lore.kernel.org/git/pull.680.git.1597857408.gitgitgadget@gmail.com/
Updates in v3
Updates since v2
Dropped "fetch: optionally allow disabling FETCH_HEAD update"
A lot of fallout from the change in the option parsing in v3 of Maintenance II.
Dropped the "verify, and delete and rewrite on failure" logic from the incremental-repack task. This might be added again later after it can be tested more thoroughly.
Updates since v1 (of this series)
PATCH 1 ("fetch: optionally allow disabling FETCH_HEAD update") was rewritten on-list. Getting a version out with this patch is the main reason for rolling a v2. (That, and Part I is re-rolled with a v2 and I want to make sure this series applies cleanly.)
The 'prefetch' and 'loose-objects' tasks had some review, but my proposed changes were not acked, so they may need another review.
UPDATES since v3 of [1]
The biggest change here is the use of "test_subcommand", based on Jonathan Nieder's approach. This requires having the exact command-line figured out, which now requires spelling out all --no- options. I also added a bunch of "2>/dev/null" checks because of the isatty(2) calls. Without that, the behavior will change depending on whether the test is run with -x/-v or without.
The 0x7FFF/0x7FFFFFFF constant problem is fixed with an EXPENSIVE test that verifies it.
The option parsing has changed to use a local struct and pass that struct to the helper methods. This is instead of having a global singleton.
Thanks,
-Stolee
Cc: sandals@crustytoothpaste.net, steadmon@google.com, jrnieder@gmail.com, peff@peff.net, congdanhqx@gmail.com, phillip.wood123@gmail.com, emilyshaffer@google.com, sluongng@gmail.com, jonathantanmy@google.com
cc: Jonathan Tan jonathantanmy@google.com
cc: Derrick Stolee stolee@gmail.com