The maintenance tasks will never run if maintenance.lock is accidentally not deleted

The maintenance tasks will never run if maintenance.lock is accidentally not deleted

[刘钟博]

In my work, I found that the prefetch task of maintenance often
failed, causing the fetch command to take a long time to execute in
monorepo.

After investigation, it was found that the maintenance.lock file was
not deleted correctly for various reasons, resulting in the inability
to trigger subsequent maintenance tasks.

The maintenance task needs to be executed in the background for a long
time. Therefore, due to various reasons, the maintenance.lock file is
easily not deleted when the process exits abnormally.
Unlike ref.lock, which will have an error prompt in the fetch command,
maintenance will only fail silently, which makes it difficult for most
people to find out where the problem is.

So is it recommended to add some mechanism to ensure that
maintenance.lock can be correctly restored when it is not deleted? For
example, add pid information to maintenance.lock, or add a lock
timeout mechanism.

I'm not sure if I missed any information, but if this is feasible, I
would be happy to contribute such a patch.

Thanks.
liuzhongbo

Re: The maintenance tasks will never run if maintenance.lock is accidentally not deleted

[Derrick Stolee]

On 9/19/24 8:40 AM, 刘钟博 wrote:
> In my work, I found that the prefetch task of maintenance often
> failed, causing the fetch command to take a long time to execute in
> monorepo.
> 
> After investigation, it was found that the maintenance.lock file was
> not deleted correctly for various reasons, resulting in the inability
> to trigger subsequent maintenance tasks.

This is unfortunately a common occurrence. It seems to be related to
the Git background processes being killed in such a way that does not
allow the standard lock cleanup mechanism to kick in.

At least, I haven't been able to find a reason why Git would be
failing with something like a segfault which would also cause leftover
.lock files.

> So is it recommended to add some mechanism to ensure that
> maintenance.lock can be correctly restored when it is not deleted? For
> example, add pid information to maintenance.lock, or add a lock
> timeout mechanism.

I can speak from experience of previously having a lock timeout
that this could cause problems where maintenance processes start
running on the same repo concurrently. The reason for this in the
past was due to being blocked on credential manager prompts.

I was vaguely remembering fixing that issue with credential prompts,
but then realized the change was only made to microsoft/git [1]. That
same change reverted the removal of "stale" .lock files.

I should put this together for an upstream patch series, finally.

[1] https://github.com/microsoft/git/pull/598

> I'm not sure if I missed any information, but if this is feasible, I
> would be happy to contribute such a patch.
I will CC you on my submission for the credential changes, as a way
to help introduce you to the code in this area.

Thanks,
-Stolee

Re: The maintenance tasks will never run if maintenance.lock is accidentally not deleted

[刘钟博]

Thank you for your response.
On Thu, Sep 19, 2024 at 8:56 PM Derrick Stolee <stolee@gmail.com> wrote:
> At least, I haven't been able to find a reason why Git would be
> failing with something like a segfault which would also cause leftover
> .lock files.
Yes, it is not necessarily a problem caused by git failure. I think it
is a natural
shortcoming of file existence lock, which cannot guarantee that the lock will be
released when the process exits abnormally.

> I can speak from experience of previously having a lock timeout
> that this could cause problems where maintenance processes start
> running on the same repo concurrently.
> [1] https://github.com/microsoft/git/pull/598
I read your commit and explored more. Perhaps the file locks provided by the
systems are a better choice, such as fcntl() on POSIX and LockFileEx()
on Windows.
They can be automatically released when the process exits abnormally.
If there are
no objections, I'll give it a try and send a patch in a few days.

Thanks
Liuzhongbo

Re: The maintenance tasks will never run if maintenance.lock is accidentally not deleted

[Derrick Stolee]

On 9/23/24 6:26 AM, 刘钟博 wrote:
 > Thank you for your response.
 > On Thu, Sep 19, 2024 at 8:56 PM Derrick Stolee <stolee@gmail.com> wrote:
 >> At least, I haven't been able to find a reason why Git would be
 >> failing with something like a segfault which would also cause leftover
 >> .lock files.
 > Yes, it is not necessarily a problem caused by git failure. I think it
 > is a natural
 > shortcoming of file existence lock, which cannot guarantee that the lock will be
 > released when the process exits abnormally.
 >
 >> I can speak from experience of previously having a lock timeout
 >> that this could cause problems where maintenance processes start
 >> running on the same repo concurrently.
 >> [1] https://github.com/microsoft/git/pull/598
 > I read your commit and explored more. Perhaps the file locks provided by the
 > systems are a better choice, such as fcntl() on POSIX and LockFileEx()
 > on Windows.
 > They can be automatically released when the process exits abnormally.
 > If there are
 > no objections, I'll give it a try and send a patch in a few days.

I'm curious to see how this would be implemented across platforms.

I think the maintenance.lock file is a particular case where such a
change would be welcome, because we are not writing to it and then doing
a rename over the non-.lock version (like we do with many other files).

For that reason, you would be creating a new API construct, not
replacing the existing lockfile API.

Thanks,
-Stolee