Skip to content

Allow configuration for loose-objects maintenance task #1885

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

Conversation

derrickstolee
Copy link

@derrickstolee derrickstolee commented Mar 21, 2025

The loose-objects task for the git maintenance run command has a hard-coded limit. The limit exists by default for the purposes of background maintenance, but can be misleading if users truly want to clean up all loose objects in one command (and don't want to use git repack). This adds a new maintenance.loose-objects.batchSize config option to help users adjust this value up or down.

When testing, I noticed that progress indicators were not always provided when isatty(2) is false. This is because the --[no-]quiet option was not appropriately passing to child processes. A small change fixes this before the config is added, so we can test the results using stderr output.

Thanks,

  • Stolee

cc: [email protected]

The --no-quiet option for 'git maintenance run' is supposed to indicate
that progress should happen even while ignoring the value of isatty(2).
However, Git implicitly asks child processes to check isatty(2) since
these arguments are not passed through.

The pass through of --no-quiet will be useful in a test in the next
change.

Signed-off-by: Derrick Stolee <[email protected]>
The 'loose-objects' task of 'git maintenance run' first deletes loose
objects that exit within packfiles and then collects loose objects into
a packfile. This second step uses an implicit limit of fifty thousand
that cannot be modified by users.

Add a new config option that allows this limit to be adjusted or ignored
entirely.

While creating tests for this option, I noticed that actually there was
an off-by-one error due to the strict comparison in the limit check. I
considered making the limit check turn true on equality, but instead I
thought to use INT_MAX as a "no limit" barrier which should mean it's
never possible to hit the limit. Thus, a new decrement to the limit is
provided if the value is positive. (The restriction to positive values
is to avoid underflow if INT_MIN is configured.)

Signed-off-by: Derrick Stolee <[email protected]>
@derrickstolee derrickstolee self-assigned this Mar 21, 2025
@derrickstolee
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Mar 24, 2025

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1885/derrickstolee/loose-objects-batch-size-v1

To fetch this version to local tag pr-1885/derrickstolee/loose-objects-batch-size-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1885/derrickstolee/loose-objects-batch-size-v1

@@ -61,6 +61,11 @@ maintenance.loose-objects.auto::
loose objects is at least the value of `maintenance.loose-objects.auto`.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Derrick Stolee via GitGitGadget" <[email protected]> writes:

> From: Derrick Stolee <[email protected]>
>
> The 'loose-objects' task of 'git maintenance run' first deletes loose
> objects that exit within packfiles and then collects loose objects into

"exit" -> "exist"?  It may read better to also do "collects" ->
"collects remaining".

> a packfile. This second step uses an implicit limit of fifty thousand
> that cannot be modified by users.
>
> Add a new config option that allows this limit to be adjusted or ignored
> entirely.
>
> While creating tests for this option, I noticed that actually there was
> an off-by-one error due to the strict comparison in the limit check.

Ahh, I, contrary to my usual routine, started reading from the code
change before reading the proposed log message and was wondering
about this exact point.  

> +	/* If batch_size is INT_MAX, then this will return 0 always. */

Cute ;-).

>  	return ++(d->count) > d->batch_size;
>  }

Copy link

gitgitgadget bot commented Mar 24, 2025

This patch series was integrated into seen via git@e986d90.

@gitgitgadget gitgitgadget bot added the seen label Mar 24, 2025
Copy link

gitgitgadget bot commented Mar 25, 2025

This branch is now known as ds/maintenance-loose-objects-batchsize.

Copy link

gitgitgadget bot commented Mar 25, 2025

This patch series was integrated into seen via git@92d1e7d.

Copy link

gitgitgadget bot commented Mar 26, 2025

There was a status update in the "New Topics" section about the branch ds/maintenance-loose-objects-batchsize on the Git mailing list:

The job to coalesce loose objects into packfiles in "git
maintenance" now has configurable batch size.

Will merge to 'next'?
source: <[email protected]>

Copy link

gitgitgadget bot commented Mar 28, 2025

This patch series was integrated into seen via git@3fe8b45.

Copy link

gitgitgadget bot commented Mar 29, 2025

This patch series was integrated into seen via git@df06b9c.

Copy link

gitgitgadget bot commented Apr 1, 2025

This patch series was integrated into seen via git@1df0e66.

Copy link

gitgitgadget bot commented Apr 7, 2025

There was a status update in the "Cooking" section about the branch ds/maintenance-loose-objects-batchsize on the Git mailing list:

The job to coalesce loose objects into packfiles in "git
maintenance" now has configurable batch size.

Will merge to 'next'.
source: <[email protected]>

Copy link

gitgitgadget bot commented Apr 7, 2025

This patch series was integrated into seen via git@f1af359.

Copy link

gitgitgadget bot commented Apr 7, 2025

This patch series was integrated into next via git@a4e55af.

@gitgitgadget gitgitgadget bot added the next label Apr 7, 2025
Copy link

gitgitgadget bot commented Apr 8, 2025

This patch series was integrated into seen via git@b4ec3df.

Copy link

gitgitgadget bot commented Apr 8, 2025

This patch series was integrated into seen via git@a4e55af.

Copy link

gitgitgadget bot commented Apr 8, 2025

This patch series was integrated into seen via git@cafc5ca.

Copy link

gitgitgadget bot commented Apr 11, 2025

This patch series was integrated into seen via git@768601a.

Copy link

gitgitgadget bot commented Apr 11, 2025

There was a status update in the "Cooking" section about the branch ds/maintenance-loose-objects-batchsize on the Git mailing list:

The job to coalesce loose objects into packfiles in "git
maintenance" now has configurable batch size.

Will merge to 'master'.
source: <[email protected]>

Copy link

gitgitgadget bot commented Apr 14, 2025

This patch series was integrated into seen via git@322169d.

Copy link

gitgitgadget bot commented Apr 15, 2025

This patch series was integrated into seen via git@d690c44.

Copy link

gitgitgadget bot commented Apr 15, 2025

This patch series was integrated into master via git@d690c44.

Copy link

gitgitgadget bot commented Apr 15, 2025

This patch series was integrated into next via git@d690c44.

@gitgitgadget gitgitgadget bot added the master label Apr 15, 2025
@gitgitgadget gitgitgadget bot closed this Apr 15, 2025
Copy link

gitgitgadget bot commented Apr 15, 2025

Closed via d690c44.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant