Update tests to record the number of events fired post-setup and only
assert for events fired during BatchRefUpdate.execute. For tests which
use writeLooseRef to setup refs, create new tests which assert the
number of RefsChangedEvent(s) rather than updating the existing ones
to call RefDirectory.exactRef as it changes the code path.
Change-Id: I0187811628d179d9c7e874c9bb8a7ddb44dd9df4
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.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>
Java GC evicts all SoftReferences when the used heap size comes close to
the maximum heap size. This means peaks in heap memory consumption can
flush the complete WindowCache which was observed to have negative
impact on performance of upload-pack in Gerrit.
Hence add a boolean option core.packedGitUseStrongRefs to allow using
strong references to reference packfile pages cached in the WindowCache.
If this option is set to true Java gc can no longer flush the
WindowCache to free memory if the used heap comes close to the maximum
heap size. On the other hand this provides more predictable performance.
Bug: 553573
Change-Id: I9de406293087ab0fa61130c8e0829775762ece8d
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Add the following statistics
- cache hit count and hit ratio
- cache miss count and miss ratio
- count of successful and failed loads
- rate of failed loads
- load, eviction and request count
- average and total load time
Use LongAdder instead of AtomicLong to implement counters in order to
improve scalability.
Optionally expose these metrics via JMX, they are registered with the
platform MBean server if the config option jmx.WindowCacheStats = true
in the user or system level git config.
Bug: 553573
Change-Id: Ia2d5246ef69b9c2bd594a23934424bc5800774aa
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Older JGit stored only milliseconds timestamps in the index. Newer
JGit may get finer timestamps from the file system. This leads to
slow index diffs when a new JGit runs against an index produced
by older JGit because many timestamps will differ and JGit will
then do many content checks. See [1].
Handle this migration case by only comparing milliseconds if the
index entry has only millisecond precision.
The inverse may also occur; also compare only milliseconds if the
file timestamp has only millisecond precision.
Do the same also for microsecond resolution. On Windows, NTFS may
provide 100ns resolution and may be used by external programs writing
the index, but Java's WindowsFileAttributes may provide only
microseconds.
File timestamp precision in Java depends not only on the Java APIs
used by different JGit versions but may also change when running the
same Java code on different VMs. And of course the resolution may
vary among operating and file systems. Moreover, timestamp precision
in the index depends on the program that wrote the index. Canonical
git may use a different resolution, maybe even different between git
versions.
[1] https://www.eclipse.org/forums/index.php/t/1100344/
Change-Id: Idfd08606c883cb98787b2138f9baf0cc89a57b56
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Using buildifier from master branch and the command:
$ buildifier -r -lint fix -warnings all .
Change-Id: I19c8ff183081093cb73bed7221a78a91b6cba4dc
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
* stable-5.0:
Prepare 4.11.10-SNAPSHOT builds
JGit v4.11.9.201909030838-r
Bazel: Update bazlets to the latest master revision
Bazel: Remove FileTreeIteratorWithTimeControl from BUILD file
BatchRefUpdate: repro racy atomic update, and fix it
Delete unused FileTreeIteratorWithTimeControl
Fix RacyGitTests#testRacyGitDetection
Change RacyGitTests to create a racy git situation in a stable way
Silence API warnings
Change-Id: I172136a031ff0730e575327cafb3527c9650a71d
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.11:
Prepare 4.11.10-SNAPSHOT builds
JGit v4.11.9.201909030838-r
Bazel: Update bazlets to the latest master revision
Bazel: Remove FileTreeIteratorWithTimeControl from BUILD file
BatchRefUpdate: repro racy atomic update, and fix it
Delete unused FileTreeIteratorWithTimeControl
Fix RacyGitTests#testRacyGitDetection
Change RacyGitTests to create a racy git situation in a stable way
Silence API warnings
Change-Id: Ifb6a4dbea2f48fd2ffa66eb737d61920aefedfbd
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.10:
Bazel: Update bazlets to the latest master revision
Bazel: Remove FileTreeIteratorWithTimeControl from BUILD file
BatchRefUpdate: repro racy atomic update, and fix it
Delete unused FileTreeIteratorWithTimeControl
Fix RacyGitTests#testRacyGitDetection
Change RacyGitTests to create a racy git situation in a stable way
Silence API warnings
Change-Id: If672b4f0c350f4e8ff7e1e706485cffd8137236d
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* stable-4.9:
BatchRefUpdate: repro racy atomic update, and fix it
Delete unused FileTreeIteratorWithTimeControl
Fix RacyGitTests#testRacyGitDetection
Change RacyGitTests to create a racy git situation in a stable way
Silence API warnings
Change-Id: Id5bf44645655fca40ad22bb1f1ad20a7c2e8f6db
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
FileTreeIteratorWithTimeControl was deleted in a024759, but was
not removed from the BUILD file, thus causing the bazel build to
fail.
Change-Id: I892c0ffcac947298d0d6009374ee2c5d9afefb66
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
(cherry picked from commit e54fde8616)
PackedBatchRefUpdate was creating a new packed-refs list that was
potentially unsorted. This would be papered over when the list was
read back from disk in parsePackedRef, which detects unsorted ref
lists on reading, and sorts them. However, the BatchRefUpdate also
installed the new (unsorted) list in-memory in
RefDirectory#packedRefs.
With the timestamp granularity code committed to stable-5.1, we can
more often accurately decide that the packed-refs file is clean, and
will return the erroneous unsorted data more often. Unluckily timed
delays also cause the file to be clean, hence this problem was
exacerbated under load.
The symptom is that refs added by a BatchRefUpdate would stop being
visible directly after they were added. In particular, the Gerrit
integration tests uses BatchRefUpdate in its setup for creating the
Admin group, and then tries to read it out directly afterward.
The tests recreates one failure case. A better approach would be to
revise RefList.Builder, so it detects out-of-order lists and
automatically sorts them.
Fixes https://bugs.eclipse.org/bugs/show_bug.cgi?id=548716 and
https://bugs.chromium.org/p/gerrit/issues/detail?id=11373.
Bug: 548716
Change-Id: I613c8059964513ce2370543620725b540b3cb6d1
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The only usage of this test iterator was removed in df637928d. Hence
delete this iterator and associated test.
Change-Id: I47710133ec3edc675c21db210960c024982668c6
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
(cherry picked from commit a024759cf5)
This test case assumed file system timestamp resolution of 1 second. On
filesystems with a finer resolution this test fails since the index
entry is only smudged if the file index entry's lastModified and the
lastModified of the git index itself are within the same filesystem
timer tick. Fix this by ensuring that these timestamps are identical
which should work for any filesystem timer resolution.
Bug: 548188
Change-Id: Id84d59e1cfeb48fa008f8f27f2f892c4f73985de
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
(cherry picked from commit 1159f9dd7c)
By using File#setLastModified, we can create a racy git situation
stably.
Tested with --runs_per_test=100
Bug: 526111
Change-Id: I60b3632d353e19f335668325aa603640be423f58
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
(cherry picked from commit df637928d2)
Move the handling of cached user and system config to getSystemConfig
and getUserConfig methods and revert the implementation of
openSystemConfig and openUserConfig to the old stateless
implementation.
This ensures the open methods respect the passed-in parent config, which
may be different on each invocation. Additionally, returning a new
instance matches the behavior of the previous implementation of the
default system reader, which downstream callers may be depending on.
Move the implementation of the new caching methods getSystemConfig and
getUserConfig up to SystemReader. This avoids that we break the ABI for
subclasses of SystemReader.
Also see [1] which fixed a similar problem with Gerrit's custom
SystemReader.
[1] https://gerrit-review.googlesource.com/c/gerrit/+/225458
Change-Id: If54a2491932d8fc914d4649cb73c9e837c5b8ad0
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
So far the git configuration and the system wide git configuration were
always reloaded when jgit accessed these global configuration files to
access global configuration options which are not in the context of a
single git repository. Cache these configurations in SystemReader and
only reload them if their file metadata observed using FileSnapshot
indicates a modification.
Change-Id: I092fe11a5d95f1c5799273cacfc7a415d0b7786c
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Tests shall not modify ~/.gitconfig. When running tests with bazel this
test failed since bazel isolates tests in a sandbox.
Change-Id: I7dd092afd14972da58a95eb7c200d353f0959fa1
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
The method org.eclipse.jgit.util.FS.supportsAtomicCreateNewFile()
should default to true as mentioned in docs [1]
org.eclipse.jgit.util.FS_POSIX.supportsAtomicCreateNewFile() method
will set the value to false if the git config
core.supportsatomiccreatenewfile is not set.
It should default to true if the configuration is undefined.
[1]
4169a95a65/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java (L372)
Bug: 544164
Change-Id: I16ccf989a89da2cf4975c200b3228b25ba4c0d55
Signed-off-by: Vishal Devgire <vishaldevgire@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
It missed to call the setup() method of its super class which prepares
the MockSystemReader
Change-Id: I39858749f8d0115fc6ac7edc8847ffb2bbc85c33
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
If we use the default system reader FileStoreAttributes cannot persist
attributes in userConfig when tests run in Bazel due to sandboxing.
Hence we need to ensure that all tests use MockSystemReader (and
especially a mocked userConfig).
Change-Id: Ic1ad8e2ec5a150c5433434a5f6667d6c4674c87d
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This was enabled unintentionally in 06fc6c7c and spams the test logs. We
can enable this when needed.
Change-Id: I9f3042c0e285ff236be65fcc02bdcfdb90efc3af
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
- use slf4j-simple for logging in test runs
- for log configuration see
https://www.slf4j.org/api/org/slf4j/impl/SimpleLogger.html
Change-Id: I9f0a532644b31162c867cd0d63f083296eaf6be5
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
- use FS.DETECTED instead of db.getFS() since the ssh config is
typically in a different place than the repository, the same is used in
OpenSshConfig
- reduce unnecessary repeated writes by introducing wait for one tick of
the file time resolution
Change-Id: Ifac915e97ff420ec5cf8e2f162e351f9f51b6b14
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn