Avoid having to scan over ALL loose refs to determine if the
name is nested within or is a container of an existing reference.
This can get really expensive if there are too many loose refs.
Instead use exactRef and getRefsByPrefix which scan based on a
prefix.
With a simple shell script(like below) using jgit client to create
1k refs in a new repository on NFS, this change brings down the time
from 12mins to 7mins.
for ref in $(seq 1 1000); do
jgit branch "$ref"
done
Here are few recorded elapsed times to create a new branch on NFS
based repositories with varying loose refs count. As we see here,
this change improves the name conflicting check from O(n^2) to O(1).
loose_refs_count with_change without_change
50 44 ms 164 ms
300 45 ms 1193 ms
1k 38 ms 2610 ms
2k 44 ms 6003 ms
9k 46 ms 27860 ms
20k 45 ms 48591 ms
50k 51 ms 135471 ms
110k 43 ms 294252 ms
160k 52 ms 430976 ms
Change-Id: Ie994fc184b8f82811bfb37b111eb9733dbe3e6e0
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>
Don't create the stream eagerly in lock(); that may cause JGit to
exceed OS or JVM limits on open file descriptors if many locks need
to be created, for instance when creating many refs. Instead create
the output stream only when one really needs to write something.
Bug: 573328
Change-Id: If9441ed40494d46f594a896d34a5c4f56f91ebf4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Don't create the stream eagerly in lock(); that may cause JGit to
exceed OS or JVM limits on open file descriptors if many locks need
to be created, for instance when creating many refs. Instead create
the output stream only when one really needs to write something.
Bug: 573328
Change-Id: If9441ed40494d46f594a896d34a5c4f56f91ebf4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Add new constructors to PackFile to improve a common use case where
callers know the directory, id, and extension, but previously needed to
construct a valid file name (with prefix, '.', etc) to create a
PackFile. Most callers can use the variant that has id as an ObjectId,
but provide an id as String variant too.
Change-Id: I39e4466abe8c9509f5916d5bfe675066570b8585
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
It's easier to follow the logic here when we can use our own objects
instead of Strings.
Change-Id: I6a166edcc67903fc1ca3544f458634c4cef8fde7
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
This class already looked very much like an Enum, but wasn't one.
As an Enum, we can use PackExt in EnumMaps and EnumSets. Convert the
Map key usage in PackDirectory to an EnumMap.
Change-Id: Ice097fd468a05805f914e6862fbd1d96ec8c45d1
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
Restore preserved packs during missing object seeks
Provide a recovery path for objects being referenced during the pack
pruning race. Due to the pack pruning race, it is possible for objects
to become referenced after a pack has been deemed safe to prune, but
before it actually gets pruned. If this happened previously, the newly
referenced objects would be missing and potentially result in a
corrupted ref.
Add the ability to recover from this situation when an object is missing
but happens to still be available in a pack in the "preserved"
directory. This is likely only useful when used in conjunction with the
--preserve-old-packs GC option, which prunes packs by hard-linking to
the preserved directory. If an object is missing and found in a pack in
the preserved directory, immediately recover that pack and its
associated files (idx, bitmaps...) by moving them back to the original
pack directory, and then retry the operation that would have failed due
to the missing object. This retry can now succeed and the repository
may avoid corruption. This approach should drastically reduce the
chance of a corrupt repository during pack pruning at very little extra
cost. This extra cost should only be incurred when objects are missing
and a failure would normally occur.
Change-Id: I2a704e3276b88cc892159d9bfe2455c6eec64252
Signed-off-by: Martin Fick <quic_mfick@quicinc.com>
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
Pack: Replace extensions bitset with bitmapIdx PackFile
The only extension that was ever consulted from the bitmap was the
bitmap index. We can simplify the Pack code as well as the code of
all the callers if we focus on just that usage.
Change-Id: I799ddfdee93142af67ce5081d14a430d36aa4c15
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
PackDirectory: Use PackFile to ensure we find preserved packs
Update scanPacksImpl and listPackDirectory (renamed to
getPackFilesByExtById) to use the new PackFile functionality to
validate file names and complete pack file sets (.pack, .idx, etc).
Most importantly, this allows a later change to rely on scanPacks() to
complete a packList that contains packs with the 'old-' prefix in their
extension.
This also eliminates duplication of logic for how to identify and
construct pack files.
Change-Id: I7175e5fefb187a29e0a7cf53c392aee922314f31
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
GC has several places where it tries to build files names for packs that
we can use the PackFile class for instead.
Change-Id: I99e5ceff9050f8583368fca35279251955e4644d
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
The PackFile class is intended to be a central place to do all
common pack filename manipulation and parsing to help reduce repeated
code and bugs. Use the PackFile class in the Pack class and in many
tests to ensure it works well in a variety of situations. Later changes
will expand use of PackFiles to even more areas.
Change-Id: I921b30f865759162bae46ddd2c6d669de06add4a
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
In a distributed setting, one can have multiple datacenters use
reftables for serving, while the ground truth for the Ref database is
administered centrally. In this setting, replication delays combined
with compaction can cause update-index ranges to overlap.
Such a setting is used at Google, and the JGit code already handles
this correctly (modulo a bugfix that applied in change I8f8215b99a).
Remove the restriction that was applied at FileReftableDatabase.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Change-Id: I6f9ed0fbd7fbc5220083ab808b22a909215f13a9
init: allow specifying the initial branch name for the new repository
Add option --initial-branch/-b to InitCommand and the CLI init command.
This is the first step to implement support for the new option
init.defaultBranch. Both were added to git in release 2.28.
See https://git-scm.com/docs/git-init#Documentation/git-init.txt--bltbranch-namegt
Bug: 564794
Change-Id: Ia383b3f90b5549db80f99b2310450a7faf6bce4c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
`copy` is documented as possibly returning a smaller number of bytes
than requested. In practice, this can occur if a block is cached and the
reader never pulls in the file to check its size.
Bug: 565874
Change-Id: I1e53b3d2f4ab09334178934dc0ef74ea99045cd3
Signed-off-by: wh <wh9692@protonmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Pack better represents the purpose of the object and paves the way to
add a PackFile object that extends File.
Change-Id: I39b4f697902d395e9b6df5e8ce53078ce72fcea3
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
Field updateHead can be a local variable in RefDirectoryRename
Keeping the field updateDate is unecessary, as it is set and used only
in the doRename method.
Change-Id: I1cdd1adf759b75c103480db7a74cec8c2d78b794
Signed-off-by: Lars Vogel <Lars.Vogel@vogella.com>
Fix FileRepository#convertToReftable which failed if no reflog existed
Deleting non-existing files when converting to reftable without backup
caused convertToReftable to fail. Observed this on a mirrored repository
which had no reflogs. Fix this by skipping missing files during
deletion.
Change-Id: I3bb913d5bfddccc6813677b873006efb849a6ebc
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Add getsRefsByPrefixWithSkips (excluding prefixes) to ReftableDatabase
We sometimes want to get all the refs except specific prefixes,
similarly to getRefsByPrefix that gets all the refs of a specific
prefix.
We now create a new method that gets all refs matching a prefix except a
set of specific prefixes.
One use-case is for Gerrit to be able to get all the refs except
refs/changes; in Gerrit we often have lots of refs/changes, but very
little other refs. Currently, to get all the refs except refs/changes we
need to get all the refs and then filter the refs/changes, which is very
inefficient. With this method, we can simply skip the unneeded prefix so
that we don't have to go over all the elements.
RefDirectory still uses the inefficient implementation, since there
isn't a simple way to use Refcursor to achieve the efficient
implementation (as done in ReftableDatabase).
Signed-off-by: Gal Paikin <paiking@google.com>
Change-Id: I8c5db581acdeb6698e3d3a2abde8da32f70c854c
Wrap the Files.list returned Stream in a try-with-resources block
Adds a new FileUtils.hasFiles(Path) helper method to correctly handle
the Files.list returned Stream.
These errors were found by compiling the code using JDK11's
javac compiler.
Change-Id: Ie8017fa54eb56afc2e939a2988d8b2c5032cd00f
Signed-off-by: Terry Parker <tparker@google.com>
This method will be used by the follow-up change. This useful if we want
to go over all the changes after a specific ref.
For example, the new method allows us to create a follow-up that would
go over all the refs until we reach a specific ref (e.g refs/changes/),
and then we use seekPastPrefix(refs/changes/) to read the rest of the refs,
thus basically we return all refs except a specific prefix.
When seeking past a prefix, the previous condition that created the
RefCursor still applies. E.g, if the cursor was created by
seekRefsWithPrefix, we can skip some refs but we will not return refs
that are not starting with this prefix.
Signed-off-by: Gal Paikin <paiking@google.com>
Change-Id: I2c02e89c877fe90da8619cb8a4a9a0c865f238ef
In some circumstances (eg. compacting a stack that has deletions), the
result may have a {min, max} range that already exists. In these
cases, we would rename onto an already existing file, which does not
work on Windows. By adding a random suffix, we disambiguate the files,
and avoid this failure scenario.
Change-Id: I0273f99bb845cfbdbd8cdd582b55d3c310505d29
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
[spotbugs]: Fix potential NPE in FileSnapshot constructor
File#getParent can return null which caused this spotbugs warning.
FS.FileStoreAttributes#get already gets the parent directory if the
passed File is not a directory and checks for null. Hence there is no
need to get the parent directory in the FileSnapshot constructor.
Change-Id: I77f71503cffb05970ab8d9ba55b69c96c53098b9
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
FileSnapshot: don't try to read file attributes twice
If file doesn't exist set state to MISSING_FILE immediately. Doing that
by calling File#lastModified and File#length effectively does the same
since they set the value to 0 if the file doesn't exist.
Log an error if a different exception than NoSuchFileException is
caught.
Change-Id: I0d4396b9f80446692a088d17522d64f735ce6708
[spotbugs] Fix FileReftableStack#equals to check for null
This fixes spotbugs warning NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT.
This implementation violated the contract defined by
java.lang.Object.equals() because it did not check for null being passed
as the argument. All equals() methods should return false if passed a
null value.
Change-Id: I607f6979613d390aae2f3546b587f63133d6d73c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
[spotbugs] FileReftableDatabase: extract lock to local variable
This fixes UL_UNRELEASED_LOCK_EXCEPTION_PATH raised by spotbugs in
#compactFully.
Change-Id: I370578ad9a027c5c9709d60a1dfafdac0cfca908
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
[spotbugs] DfsReftableDatabase: extract lock to local variable
This fixes UL_UNRELEASED_LOCK_EXCEPTION_PATH raised by spotbugs in
#DfsReftableDatabase and #clearCache.
Change-Id: Ifd3189288d2a8e64139c02cd105eb335fa2f68cf
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
[spotbugs] Fix potential NPE in FileRepository#convertToReftable
File#listFiles can return null. Use Files#list which does not return
null and should be faster since it's returning directory entries lazily
while File#listFiles fetches them eagerly.
Change-Id: I3bfe2a52278244fc469143692c06b05d9af0d0d4
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This was experimental code and never used in production.
Change-Id: Ia3da7f2b82d9e365cec2ccf9397cbc47439cd150
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Split out loose object handling from ObjectDirectory
The ObjectDirectory class manages the interactions for the entire object
database, this includes loose objects, packfiles, alternates, and
shallow commits. To help reduce the complexity of this class, abstract
some of the loose object specific details into a class which understands
just this, leaving the ObjectDirectory to focus more on the interactions
between the different mechanisms.
Change-Id: I39f3a74d6308f042a2a2baa57769f4acde5ba5e0
Signed-off-by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The ObjectDirectory class manages the interactions for the entire object
database, this includes loose objects, packfiles, alternates, and
shallow commits. To help reduce the complexity of this class, abstract
some of the packfile specific details into a class which understands
just this, leaving the ObjectDirectory to focus more on the interactions
between the different mechanisms.
Change-Id: I5cc87b964434b0afa860b3fe23867a77b3c3a4f2
Signed-off-by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Fix PackInvalidException when fetch and repack run concurrently
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 -
afef866a44
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 the
try block and still having the invalid field set to true. I did
exhaustive testing of the change running concurrent clones and pushes
indefinitely and with the patch applied it never fails while without the
patch it takes relatively short to get the error.
See: https://www.eclipse.org/lists/jgit-dev/msg04014.html
Bug: 569349
Change-Id: I9dbf8801c8d3131955ad7124f42b62095d96da54
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
GC#deleteOrphans: handle failure to list files in pack directory
- log an error
- either there is no list or it is incomplete hence return immediately
Change-Id: Ieee5378ca06304056b9ccc30c1acd5f52360052d
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If pack or index files are guarded by a pack lock (.keep file)
deleteOrphans() should not touch the respective files protected by the
lock file. Otherwise it may interfere with PackInserter concurrently
inserting a new pack file and its index.
The problem was caused by the following race.
All mentioned files are located in "objects/pack/".
File endings relevant in "pack" dir:
.pack
.keep
.idx
.bitmap
When ReceivePack receives a pack file it executes the following steps:
ReceivePack.service():
receivePackAndCheckConnectivity():
receivePack():
receive the pack
parse the pack, returns packLock (.keep file)
PackInserter.flush():
write tmpPck file: "insert_<random>.pack"
write tmpIdx file: "insert_<random>.idx"
real pack name: "pack-<SHA1>.pack"
real index name: "pack-<SHA1>.idx"
atomic rename tmpPack to realPack
atomic rename tmpIdx to tmpIdx
execute commands
unlock pack by removing .keep file
trigger auto gc if enabled
When PackInserter.flush() renames the temporary pack to the final
"pack-xxx.pack" file the temporary pack index file "insert_xxx.idx"
has no matching .pack file with the same base name for a short interval.
If deleteOrphans() ran during that interval it deduced the pack index
file was orphaned. Subsequently the missing pack index caused
MissingObjectExceptions since objects contained in the pack couldn't be
looked up anymore.
Bug: https://bugs.chromium.org/p/gerrit/issues/detail?id=13544
Change-Id: I559c81e4b1d7c487f92a751bd78b987d32c98719
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Fix IOException occurring when calling
GC on a repository with absent objects/pack folder.
Change-Id: I5be1333a0726f4d7491afd25ddac85451686c30a
Signed-off-by: Nail Samatov <sanail@yandex.ru>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
DfsBundleWriter writes out the entire repository to a Git bundle file.
It packs all objects included in the packfile by concatenating all pack
files. This makes the bundle creation fast and cheap. Useful for backing
up a repository as-is.
Change-Id: Iee20e4b1ab45b2a178dde8c72093c0dd83f04805
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
Fix possible NegativeArraySizeException in PackIndexV1
Due to an integer overflow bug, the current "Index file is too large
for jgit" check did not work properly and subsequently a
NegativeArraySizeException was raised.
Change-Id: I2736efb28987c29e56bc946563b7fa781898a94a
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
The WindowCache is configured statically with a default
WindowCacheConfig. The default config says (for backwards
compatibility reasons) to publish the MBean. As a result,
the MBean always gets published.
By delaying the MBean registration until the first call to
getInstance() or get(PackFile, long) we can avoid the forced
registration and do it only if not re-configured in the meantime
not to publish the bean. (As is done by Egit, to avoid a very
early costly access to the user and system config during plug-in
activation.)
Bug: 563740
Change-Id: I8a941342c0833acee2107515e64299aada7e0520
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Do not send empty blob in response to blob:none filter
If I create a repository containing an empty file and clone it
with
git clone --no-checkout --filter=blob:none \
https://url/of/repository
then I would expect no blobs to be transferred over the wire. Alas,
JGit rewrites filter=blob:none to filter=blob:limit=0, so if the
repository contains an empty file then the empty blob gets
transferred.
Fix it by teaching JGit about filters based on object type to
complement the existing filters based on object size. This prepares
us for other future filters such as object:none.
In particular, this means we do not need to look up the size of the
filtered blobs, which should speed up clones. Noticed by Anna
Pologova and Terry Parker.
Change-Id: Id4b234921a190c108d8be2c87f54dcbfa811602a
Signed-off-by: Jonathan Nieder <jrn@google.com>
MergedReftable: Include the last reftable in determining minUpdateIndex
MergedReftable ignores the last reftable in the stack while calculating the
minUpdateIndex.
Update the loop indices to include all reftables in the minUpdateIndex
calculation, while skipping position 0 as it is read outside the loop.
Change-Id: I12d3e714581e93d178be79c02408a67ab2bd838e
Signed-off-by: Minh Thai <mthai@google.com>
PackBitmapIndex: Not buffer inflated bitmap during bitmap creation.
Currently we're buffering the inflated bitmap entry in
BasePackBitmapIndex to optimize running time. However, this will use
lots of memory during the creation of the pack bitmap index file.
And change 161456, which rewrote the entire getBitmap method, increased
the fetch latency significantly.
This commit introduces getBitmapWithoutCaching method which is used in
the pack bitmap index file creation only and aims to save memory during
garbage collection and not increase fetch latency.
Change-Id: I7b982c9d4e38f5f6193eaa03894e894ba992b33b
Signed-off-by: Yunjie Li <yunjieli@google.com>
ObjectWritingException and FileNotFoundException are subclasses
of IOException, which is already declared. Error does not need
to be explicitly declared.
Change-Id: I879820a33e10ec3a7ef676adc9c9148d2b3c4b27
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Kaushik Lingarkar
Matthias Sohn
Thomas Wolf
Nasser Grainawi
Martin Fick
Han-Wen Nienhuys
wh
Lars Vogel
Gal Paikin
Terry Parker
Martin Fick
Petr Hrebejk
Nail Samatov
Masaya Suzuki
Marc Strapetz
Jonathan Nieder
Minh Thai
Yunjie Li
David Pursehouse