Suppress warning for trying to delete non-empty directory
This is actually a fairly common occurrence; deleting the parent
directories can work only if the file deleted was the last one
in the directory.
Bug: 537872
Change-Id: I86d1d45e1e2631332025ff24af8dfd46c9725711
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Since I3870cadb4, GC task was always delegated to an executor even when
background option was set to false. This was an issue because if more
than one GC object was instantiated and executed in parallel, only one GC
was actually running because of the single thread executor.
Change-Id: I8c587d22d63c1601b7d75914692644a385cd86d6
Signed-off-by: Hugo Arès <hugo.ares@ericsson.com>
Remove completely the empty directories under refs/<namespace>
including the first level partition of the changes, when they are
completely empty.
Bug: 536777
Change-Id: I88304d34cc42435919c2d1480258684d993dfdca
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Use java.nio to delete path to get detailed errors
Get the full IOException of the reason why a directory
cannot be removed during GC.
Change-Id: Ia555bce009fa48087a73d677f1ce3b9c0b685b57
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
After packaging references, the folders containing these references are
not deleted. In a busy repository, this causes operations to slow down
as traversing the references tree becomes longer.
Delete empty reference folders after the loose references have been
packed.
To avoid deleting a folder that was just created by another concurrent
operation, only delete folders that were not modified in the last 30
seconds.
Signed-off-by: Hector Oswaldo Caballero <hector.caballero@ericsson.com>
Change-Id: Ie79447d6121271cf5e25171be377ea396c7028e0
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Log as warning when an attempt to remove a directory
fails. This helps troubleshooting some bugs like the GC leaving
behind empty directories.
Change-Id: Idb94ce17f8be9668a970c7ecae31436bf434073c
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
From the javadoc for Files.list:
"The returned stream encapsulates a DirectoryStream. If timely disposal
of file system resources is required, the try-with-resources construct
should be used to ensure that the stream's close method is invoked
after the stream operations are completed."
This is the only call to Files#newDirectoryStream that is not already in
a try-with-resources.
Change-Id: I91e6c56b5d74e8435457ad6ed9e6b4b24d2aa14e
(cherry picked from commit 1c16ea4601)
The intent with the setCompressionLevel and checkExisting methods (which
are already public) is for callers to be able to call them, but they
can't do that if the class itself is not public.
Change-Id: I014044fec3bfa1d33775500345efd60eb5d45bde
PackInserter: Ensure objects are written at the end of the pack
When interleaving reads and writes from an unflushed pack, we forgot to
reset the file pointer back to the end of the file before writing more
new objects. This had at least two unfortunate effects:
* The pack data was potentially corrupt, since we could overwrite
previous portions of the file willy-nilly.
* The CountingOutputStream would report more bytes read than the size
of the file, which stored the wrong PackedObjectInfo, which would
cause EOFs during reading.
We already had a test in PackInserterTest which was supposed to catch
bugs like this, by interleaving reads and writes. Unfortunately, it
didn't catch the bug, since as an implementation detail we always read a
full buffer's worth of data from the file when inflating during
readback. If the size of the file was less than the offset of the object
we were reading back plus one buffer (8192 bytes), we would completely
accidentally end up back in the right place in the file.
So, add another test for this case where we read back a small object
positioned before a large object. Before the fix, this test exhibited
exactly the "Unexpected EOF" error reported at crbug.com/gerrit/7668.
Change-Id: I74f08f3d5d9046781d59e5bd7c84916ff8225c3b
Replace explicit calls to initCause where possible
Where the exception being thrown has a constructor that takes a
Throwable, use that instead of instantiating the exception and then
explicitly calling initCause.
Change-Id: I06a0df407ba751a7af8c1c4a46f9e2714f13dbe3
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
CorruptObjectException has a constructor that takes Throwable and
calls initCause with it. Use that instead of instantiating the
exception and explicitly calling initCause.
Change-Id: I1f2747d6c4cc5249e93401b9787eb4ceb50cb995
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
Use new StoredObjectRepresentationNotAvailableException constructor
In 5e7eed4 a new StoredObjectRepresentationNotAvailableException
constructor was added, that takes a Throwable to initialize the
exception cause.
Update more call sites to use this constructor instead of first
instantiating it and explicitly calling initCause().
All callers now use the new constructor, so annotate the other one as
deprecated.
Change-Id: I6d2a7e289a95f0360ddebf904cfd8b6c18fef10c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
StoredObjectRepresentationNotAvailableException: Add constructor that takes cause
If the cause can be passed into the constructor, callers don't need to
instantiate it and then explicitly call initCause.
Note that the constructors in this class cause "non-API parameter type"
warnings because ObjectToPack is internal, however it's probably OK
since the only non-internal reference to it is in the pgm.debug package.
Change-Id: Ia4eab24e79f9afe6214ea8160137d941d4048319
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
DfsFsck: Skip unborn branches and symrefs to nowhere
The map returned by getAllRefs includes all refs, including symrefs like
HEAD that may not point to any object yet. That is a valid state (e.g.,
in a new repository that has just been created by "git init"), so skip
such refs.
Change-Id: Ieff8a1aa738b8d09a2990d075eb20601156b70d3
Signed-off-by: Zhen Chen <czhen@google.com>
When we are cloning we have no refs at all yet, and there cannot
(or at least should not) be any other thread doing something with
refs yet.
Locking loose refs is thus not needed, since there are no loose
refs yet and nothing should be trying to create them concurrently.
Let's skip the whole loose ref locking when we are cloning a repository.
As a result, JGit will write the refs directly to the packed-refs
file, and will not create the refs/remotes/ directories nor the
lock files underneath when cloning and packed refs are used. Since
no lock files are created, any problems on case-insensitive file
systems with tag or branch names that differ only in case are avoided
during cloning.
Detect if we are cloning based on the following heuristics:
* HEAD is a dangling symref
* There is no loose ref
* There is no packed-refs file
Note, however, that there may still be problems with such tag or
branch names later on. This is primarily a five-minutes-past-twelve
stop-gap measure to resolve the referenced bug, which affects the
Oxygen.2 release.
Bug: 528497
Change-Id: I57860c29c210568165276a123b855e462b6a107a
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
DfsBlockCache.hasBlock0: quickly check for file in cache
This can be useful for sophisticated pre-read algorithms to quickly
determine if a file is likely already in cache, especially small
reftables which may be smaller than a typical DFS block size.
Change-Id: I7756948063b722ff650c9ba82060ff9ad554b0ba
InMemoryRepository: Make inner class MemObjDatabase static
FindBugs reports:
This class is an inner class, but does not use its embedded reference
to the object which created it. This reference makes the instances
of the class larger, and may keep the reference to the creator object
alive longer than necessary. If possible, the class should be made
static.
Change-Id: I9f49de32b4cd81b7ef1239b390353689263bf66e
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
DfsBlockCache#creditSpace: release clockLock in finally block
Enclose the call to getStat in a `try`, and release the previously
acquired lock in the `finally`. This prevents that the lock is left
unreleased in the case of an exception being raised in getStat.
Change-Id: I17b4cd134dae887e23a1165253be0ac2d4fd452c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
When a GC operation is interrupted, temporary packs and indexes can be
left on the pack folder. In big, busy repositories this can lead to
significant amounts of wasted disk space if this interruption is done
with a certain frequency.
Remove stale temporary packs and indexes at the end of the GC process so
they do not accumulate. To avoid interfering with a possible concurrent
JGit GC process in the same repository, only delete temporary files that
are older than one day.
Change-Id: If9b6c1e57fac8a6a0ecc0a703089634caba4caae
Signed-off-by: Hector Caballero <hector.caballero@ericsson.com>
When running on NFS there was a chance that JGits LockFile
semantic is broken because File#createNewFile() may allow
multiple clients to create the same file in parallel. This
change provides a fix which is only used when the new config
option core.supportsAtomicCreateNewFile is set to false. The
default for this option is true. This option can only be set in the
global or the system config file. The repository config file is not
taken into account in this case.
If the config option core.supportsAtomicCreateNewFile is true
then File#createNewFile() is trusted and the behaviour doesn't
change.
But if core.supportsAtomicCreateNewFile is set to false then after
successful creation of the lock file a hardlink 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.
This expensive workaround is described in
https://www.time-travellers.org/shane/papers/NFS_considered_harmful.html
section III.d) "Exclusive File Creation"
Change-Id: I3d2cc48d8eb280d5f7039eb94da37804f903be6a
Honor trustFolderStats also when reading packed-refs
Then list of packed refs was cached in RefDirectory based on mtime of
the packed-refs file. This may fail on NFS when attributes are cached.
A cached mtime of the packed-refs file could cause JGit to trust the
cached content of this file and to overlook that the file is modified.
Honor the config option trustFolderStats and always read the packed-refs
content if the option is false. By default this option is set to true
and this fix is not active.
Change-Id: I2b65cfaa8f4aba2efbf8a5e865d3f09f927e2eec
Add flag for keeping ref tombstones in GC reftable
A tombstone will prevent a delayed reference update from resurrecting the
deleted reference.
Change-Id: Id9f4df43d435a299ff16cef614821439edef9b11
Signed-off-by: Minh Thai <mthai@google.com>
ReftableCompactor should accept 0 for minUpdateIndex
Do not use 0 as the unset value for minUpdateIndex, as input reftables
may have minUpdateIndex starting at 0.
Change-Id: Ie040a6b73d4a5eba5521e51d0ee4580713c84a3e
Signed-off-by: Minh Thai <mthai@google.com>
So far, in order to get the pack directory it was necessary to resolve
it from the object directory. This resolution is already done when
creating the object directory, so simplify the call by just adding a
getter to the pack directory.
Change-Id: I69e783141dc6739024e8b3d5acc30843edd651a7
Signed-off-by: Hector Caballero <hector.caballero@ericsson.com>
When invoking File.toPath(), an (unchecked) InvalidPathException may be
thrown which should be converted to a checked IOException.
For now, we will replace File.toPath() by FileUtils.toPath() only for
code which can already handle IOExceptions.
Change-Id: I0f0c5fd2a11739e7a02071adae9a5550985d4df6
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
The object checks may take a long time and sometimes we are only
interested in connectivity check.
This is similar to 'git fsck --connectivity-only'.
Change-Id: I654e8fdccdb16d796f920088429d188cc96734bc
Signed-off-by: Zhen Chen <czhen@google.com>
Make PackWriterBitmapWriter class public and move it to a more central
location, in preparation for its use by another class (in a subsequent
commit).
One of its inner static classes, AddUnseenToBitmapFilter, previously
package-private, is also used directly in its former package. Therefore,
AddUnseenToBitmapFilter and its sibling class have been moved to an
internal package instead.
Change-Id: I740bc4bfc4e4e3c857d1ee7d25fe45e90cd22a75
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Applications that use ObjectInserters to create lots of individual
objects may prefer to avoid cluttering up the object directory with
loose objects. Add a specialized inserter implementation that produces a
single pack file no matter how many objects. This inserter is loosely
based on the existing DfsInserter implementation, but is simpler since
we don't need to buffer blocks in memory before writing to storage.
An alternative for such applications would be to write out the loose
objects and then repack just those objects later. This operation is not
currently supported with the GC class, which always repacks existing
packs when compacting loose objects. This in turn requires more
CPU-intensive reachability checks and extra I/O to copy objects from old
packs to new packs.
So, the choice was between implementing a new variant of repack, or not
writing loose objects in the first place. The latter approach is likely
less code overall, and avoids unnecessary I/O at runtime.
The current implementation does not yet support newReader() for reading
back objects.
Change-Id: I2074418f4e65853b7113de5eaced3a6b037d1a17
ObjectDirectory: Remove last modified check in insertPack
GC explicitly handles the case where a new pack has the same name as an
existing pack due to it containing the exact same set of objects. In
this case, the pack passed to insertPack will have the same name as an
existing pack, but it will also almost certainly have a later mtime than
the existing pack.
The loop in insertPack tried to short-circuit when inserting a new pack,
to avoid walking more of the pack list than necessary. Unfortunately,
this means it will never get to the check for an identical name,
resulting in a duplicate entry for the same PackFile in the pack list.
Remove the short-circuit so that insertPack does not insert a duplicate
entry.
Change-Id: I00711b28594622ad3bd104332334e8a3592cda7f
Align PackStatistics and DfsPackDescription list length
Without this the caller cannot tell which PackStatistics is for which
pack file.
Change-Id: Ifa530f8bc82459611ae694c0a0d111daec3e08f3
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
dfs: Switch InMemoryRepository to DfsReftableDatabase
This ensure DfsReftableDatabase is tested by the same test suites that
use/test InMemoryRepository. It also simplifies the logic of
InMemoryRepository and brings its compatibility story closer to any
other DFS repository that uses reftables for its reference storage.
Change-Id: I881469fd77ed11a9239b477633510b8c482a19ca
Signed-off-by: Minh Thai <mthai@google.com>
Signed-off-by: Terry Parker <tparker@google.com>
DfsReftableDatabase is a new alternative for DfsRefDatabase that
handles more operations for the implementor by delegating through
reftables. All reftable files are stored in sibling DfsObjDatabase
using PackExt.REFTABLE and PackSource.INSERT.
Its assumed the DfsObjDatabase periodically runs compactions and GCs
using DfsPackCompactor and DfsGarbageCollector. Those passes are
essential to collapsing the stack of reftables.
Change-Id: Ia03196ff6fd9ae2d0623c3747cfa84357c6d0c79
Signed-off-by: Minh Thai <mthai@google.com>
Signed-off-by: Terry Parker <tparker@google.com>
Allow creating symbolic references with link, and deleting them or
switching to ObjectId with unlink. How this happens is up to the
individual RefDatabase.
The default implementation detaches RefUpdate if a symbolic reference
is involved, supporting these command instances on RefDirectory.
Unfortunately the packed-refs file does not support storing symrefs,
so atomic transactions involving more than one symref command are
failed early.
Updating InMemoryRepository is deferred until reftable lands, as I
plan to switch InMemoryRepository to use reftable for its internal
storage representation.
Change-Id: Ibcae068b17a2fc6d958f767f402a570ad88d9151
Signed-off-by: Minh Thai <mthai@google.com>
Signed-off-by: Terry Parker <tparker@google.com>
ReflogWriter: Align auto-creation defaults with C git
Per git-config(1), core.logAllRefUpdates auto-creates reflogs for HEAD
and for refs under heads, notes, tags, and for HEAD. Add notes and
remove stash from ReflogWriter#shouldAutoCreateLog. Explicitly force
writing reflogs for refs/stash at call sites, now that this is
supported.
Change-Id: I3a46d2c2703b7c243e0ee2bbf6948279800c485c
Support force writing reflog on a per-update basis
Even if a repository has core.logAllRefUpdates=true, ReflogWriter does
not create reflog files unless the refs are under a hard-coded list of
prefixes, or unless the forceWrite bit is set. Expose the forceWrite bit
on a per-update basis in RefUpdate/BatchRefUpdate/ReceiveCommand,
creating RefLogWriters as necessary.
Change-Id: Ifc851fba00f76bf56d4134f821d0576b37810f80
Thomas Wolf
Hugo Arès
Luca Milanesio
Hector Caballero
Dave Borowitz
Matthias Sohn
David Pursehouse
Zhen Chen
Shawn Pearce
Minh Thai
Christian Halstrick
Marc Strapetz
Jonathan Tan
Masaya Suzuki