Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] Concurrent run of repack and clone sometimes fails

On Mon, Nov 30, 2020 at 4:18 PM hrebejk <hrebejk@xxxxxxxxx> wrote:
Hi,

We are running several servers with jGit. We need to run repack from time to time to keep the repos performant. I.e. after push we test how many small packs are in the repo and when a threshold is reached we run the repack.
After upgrading jGit version we've found that if someone does the clone at the time repack is running the clone sometimes (not always) fails because the repack removes .pack file used by the clone. Server exception and client error attached.

I've tracked down the cause and it seems to be introduced between jGit 5.2 (which we upgraded from) and 5.3 and being caused by this commit: Move throw of PackInvalidException outside the catch - https://github.com/eclipse/jgit/commit/afef866a44cd65fef292c174cad445b3fb526400

The problem is that when the throw was inside of the try block the last catch block catched the exception and called openFailed(false) method. It is true that it called it with invalidate = false, which is wrong. The real problem though is that with the throw outside of the try block the openFail is not called at all and the fields activeWindows and activeCopyRawData are not set to 0. Which affects the later called tests like: if (++activeCopyRawData == 1 && activeWindows == 0).

The fix for this is relatively simple keeping the throw outside of a the try block and still having the invalid field set to true. See the attached patch (against current master). I did exhaustive testing of the change running concurrent clones and pushes indefinitely and with the patch applied it never fails where without it takes relatively short to get the error. However I'm not an expert in the jGit code and I may well be wrong about how to fix this.

Thanks for finding this. Was searching for the root cause of this malbehavior already.
I pushed your patch for review to Gerrit here [1]. Please take a look if I applied it correctly
and reply on the change in Gerrit accordingly.

Find instructions how to contribute via Gerrit in the contributor guide [2].

Will now test this patch with automated Gerrit load tests.


-Matthias
 
Please let me know If you need me to do more, such as filing a bug and maybe provide a runable test case (will have to be a jGit server though)

Thanks
Hrebejk
_______________________________________________
jgit-dev mailing list
jgit-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jgit-dev

Back to the top