Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] Exception caught while accessing pack file
On Fri, Sep 6, 2019 at 12:31 PM Mincong Huang <mhuang@xxxxxxxxx> wrote:
Hi Matthias and David,

I really appreciate your help. Thank you!!

On Fri, Sep 6, 2019 at 12:31 AM Matthias Sohn <matthias.sohn@xxxxxxxxx> wrote:
On Thu, Sep 5, 2019 at 11:29 PM Mincong Huang <mincong.h@xxxxxxxxx> wrote:
Hi everyone,

I came across the error exceptionWhileReadingPack today: "ERROR: Exception caught while accessing pack file {0}, the pack file might be corrupt, {1}. Caught {2} consecutive errors while trying to read this pack." Could somebody explain in which circumstance this can happen? I saw Luca's commit message back to 2017[2], but I would like to have more detail from you.

which version of JGit do you use ? 

We are using v5.2.1.201812262042-r.

then you need to upgrade to a newer version having the fix for this issue,
the next one above 5.2.1 would be 5.3.5 released this week, see
The FileNotFoundException is typically raised in three conditions:
1. file doesn't exist
2. incompatible read vs. read/write open modes
3. filesystem locking
4. temporary lack of resources (e.g. too many open files)

1. is already managed, 2. would never happen as packs are not
overwritten while with 3. and 4. it is worth logging the exception and
retrying to read the pack again.

I suspect that our installation is in the 3rd case because we're using EFS of AWS. Any suggestion will be welcome. Thanks a lot!

In general jgit expects (on *nix) a posix compliant filesystem. NFS is not posix compliant.
NFS client side caching is causing issues when multiple jgit instances access data stored on nfs
via different nfs client instances, e.g. if multiple machines access the same shared NFS volume.
NFS does not support atomic file rename which jgit uses to implement transactions.
Switching off nfs client caching is typically too slow to be useful.


We are considering switching to a normal FS. This is still in progress...

choose a modern one with a high timestamp resolution like ext4, btrfs, xfs, zfs 
 
There are a couple of workarounds in jgit to workaround some of the nfs issues. 
Some of these workarounds need to be enabled by setting corresponding JGit options.
Note that these options have a negative impact on performance.

## NFS workarounds
- core.trustFolderStat: boolean
  - default: true
  - Whether to trust the pack folder's modification time. If set to false we will
    always scan the .git/objects/pack folder to check for new pack files. If set to
    true (default) we use the lastmodified attribute of the folder and assume that
    no new pack files can be in this folder if his modification time has not
    changed
- core.supportsAtomicFileCreation: boolean
  - supported on Posix
  - default: true
  - if core.supportsAtomicCreateNewFile = false then after
    successful creation of the lock file a hard link to that lock file is
    created and the attribute nlink of the lock file is checked to be 2. If
    multiple clients manage to create the same lock file nlink would be
    greater than 2 showing the error. The hard link needs to be retained
    until the corresponding file is no longer needed in order to prevent that
    another process can create the same file concurrently using another NFS
    client which might not yet see the file due to caching.
  - see [https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html]
 
From the source code of JGit, it seems that these configuration properties are defined in Git configuration. Three configuration sources are loaded in order for a File Repository:
  1. System Config
  2. User Config (~/.gitconfig)
  3. Repository Config ($repo/config for bare repository)
We have thousands of repositories. Do you think defining those two properties in User Config (~/.gitconfig) will work? I mean

$ git config --global core.trustFolderStat false
$ git config --global core.supportsAtomicCreateNewFile false

yes, this should work, except if you wrote your own SystemReader to load config files from other places 
 
JGit HTTP Server will be able to reload the configuration lazily without reboot, is that correct?

yes
 
At each Git repository level, JGit is able to detect outdated configure and reload lazily.

yes, this should work.
Handling of user and system level gitconfigs was improved in the releases published this week
by caching user and system level gitconfigs to reduce IO, we still check file metadata to detect
if we need to reload any of the gitconfigs
 
I also took a look into GitServlet.java, but it does not seem to be the good option.

GitServlet handles support for HTTP git protocol, configs are implemented in FileBasedConfig
 
---

By curiosity, I would like to fix the incorrect text in message exceptionWhileReadingPack, but I think Matthias already did it[3].

[3] is available since JGit release v5.1.6.201903130242-r
 
Do you think it is worth to improve the existing error message handling mechanism? I have two ideas in mind:
  1. Remove unused messages. I found 62 of them
patches welcome, unused messages should be removed since there is no need to maintain them
and removing them will reduce size of jgit binary artefact

I would like to help on this one, but I think @David also did it. Thank you @David. I reviewed one of your PRs https://git.eclipse.org/r/#/c/149009/.

yes, done by David 
  1. Refactor the code to avoid similar placeholder problems, like this one [4], so that the relationship between arguments and pattern can be easier to see.
we are close to releasing final 5.5.0, so first of all I'd like to fix all known issues in error message texts
If you have list please file a bug to track these problems, patches welcome.


I don't have such list. Scanning all message texts is not easy to achieve. I think about parsing the Java files to extract an AST, then inject the message text accordingly to see if the number of placeholders matches the number of arguments. But I feel like this is a lot of effort.

then don't waste time with that
 
Please describe your proposed refactoring or push it for review, see the contributor guide [5]
For refactoring, I think about using static methods for creating error messages, one per method. Therefore, we can see the number of input arguments vs number of placeholder in the same place. I have to dig deeper to refine it. I would like to do it myself if you're OK with it. I can submit a PR probably this weekend.
 
_______________________________________________
jgit-dev mailing list
jgit-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/jgit-dev

Best,

--

Nuxeo Logo

Mincong Huang Software Engineer Github

Nuxeo Content Services Platform. Stay ahead.

Back to the top