Matthias Sohn [Wed, 21 Aug 2019 10:13:08 +0000 (12:13 +0200)]
Merge branch 'stable-5.2' into stable-5.3
* stable-5.2:
Add missing @since tag on FileTreeIterator#getLastModifiedInstant
Prepare 5.1.10-SNAPSHOT builds
JGit v5.1.9.201908210455-r
Avoid sign extension when comparing mtime with Instant#getEpochSecond
Fix deprecation in DirCache caused by Instant based DirCacheEntry
Change-Id: If6d5f4dfd9fc8e8c09e29aa11b1004057eafeb9f Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 21 Aug 2019 10:02:22 +0000 (12:02 +0200)]
Merge branch 'stable-5.1' into stable-5.2
* stable-5.1:
Add missing @since tag on FileTreeIterator#getLastModifiedInstant
Prepare 5.1.10-SNAPSHOT builds
JGit v5.1.9.201908210455-r
Avoid sign extension when comparing mtime with Instant#getEpochSecond
Fix deprecation in DirCache caused by Instant based DirCacheEntry
Change-Id: Id824c0b8b14dad5947ae9da1f90c3471e07b400f Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Tue, 20 Aug 2019 22:29:45 +0000 (00:29 +0200)]
Avoid sign extension when comparing mtime with Instant#getEpochSecond
Ensure we use the same type when comparing seconds since the epoch.
This does not prevent that in 2038 timestamps in seconds since the epoch
stored in a 32 bit integer will overflow. Integer.MAX_VALUE translates
to 2038-01-19T03:14:07Z. After this date we'll have an issue since we
store seconds since the epoch in a 32 bit integer in some places.
Bug: 319142
Change-Id: If0c03003d40b480f044686e2f7a2f62c9f4e2fe1 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Michael Keppler [Tue, 20 Aug 2019 14:09:12 +0000 (16:09 +0200)]
Fix deprecation in DirCache caused by Instant based DirCacheEntry
Replace the two int variables smudge_s and smudge_ns by an Instant and
use the new method DirCacheEntry.mightBeRacilyClean(Instant).
Change-Id: Id70adbb0856a64909617acf65da1bae8e2ae934a Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Mon, 19 Aug 2019 21:41:04 +0000 (23:41 +0200)]
Merge branch 'stable-5.2' into stable-5.3
* stable-5.2:
Fix NPE in RebaseTodoFile#parseComments
Fix NPE in ObjectIdOwnerMap#get
Fix NPE in CommitOnlyTest#getHead
FileUtils#lastModifiedInstant should not log error if path doesn't exist
Cache user global and system-wide git configurations
Avoid setup and saving FileStoreAttributes compete for ~/.gitconfig lock
Add missing dependencies for running FS_POSIXTest in Eclipse
Fix javadoc for SystemReader#getInstance
Improve retry handling when saving FileStoreAttributes fails
Ensure FSTest uses MockSystemReader
Make supportsAtomicCreateNewFile return true as default
Update orbit to R20190602212107-2019-06 to enable backports from master
Handle InvalidPathException in FS_POSIX#createNewFileAtomic
Ensure root cause of lock creation failures is logged
Implement toString in MockSystemReader and MockConfig
LocalDiskRefTreeDatabaseTest shall use MockSystemReader
Ensure LocalDiskRepositoryTestCase#setup fully uses MockSystemReader
Ensure we use MockSystemReader in tests
Override FileBasedConfig's save method in MockConfig
Remove FileBasedConfig.load(boolean) introduced in d45219ba
Disable debug log for FS in org.eclipse.jgit.test
Bazel: enable logging for tests in org.eclipse.jgit.test
LockFile: log exception if creation of lock file failed
Stop using deprecated Constants.CHARACTER_ENCODING
Change-Id: If0c5010a2cf151ebebb2f2088fac3ee02c5007b9 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Mon, 19 Aug 2019 15:52:02 +0000 (17:52 +0200)]
Merge branch 'stable-5.1' into stable-5.2
* stable-5.1:
Fix NPE in RebaseTodoFile#parseComments
Fix NPE in ObjectIdOwnerMap#get
Fix NPE in CommitOnlyTest#getHead
FileUtils#lastModifiedInstant should not log error if path doesn't exist
Cache user global and system-wide git configurations
Avoid setup and saving FileStoreAttributes compete for ~/.gitconfig lock
Add missing dependencies for running FS_POSIXTest in Eclipse
Fix javadoc for SystemReader#getInstance
Improve retry handling when saving FileStoreAttributes fails
Ensure FSTest uses MockSystemReader
Make supportsAtomicCreateNewFile return true as default
Update orbit to R20190602212107-2019-06 to enable backports from master
Handle InvalidPathException in FS_POSIX#createNewFileAtomic
Ensure root cause of lock creation failures is logged
Implement toString in MockSystemReader and MockConfig
LocalDiskRefTreeDatabaseTest shall use MockSystemReader
Ensure LocalDiskRepositoryTestCase#setup fully uses MockSystemReader
Ensure we use MockSystemReader in tests
Override FileBasedConfig's save method in MockConfig
Remove FileBasedConfig.load(boolean) introduced in d45219ba
Disable debug log for FS in org.eclipse.jgit.test
Bazel: enable logging for tests in org.eclipse.jgit.test
LockFile: log exception if creation of lock file failed
Stop using deprecated Constants.CHARACTER_ENCODING
Change-Id: I48c585f3c9287be7d6ddb6b01a1955444e13fa31 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 14 Aug 2019 23:25:28 +0000 (01:25 +0200)]
Cache user global and system-wide git configurations
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>
Matthias Sohn [Sun, 11 Aug 2019 00:43:02 +0000 (02:43 +0200)]
Avoid setup and saving FileStoreAttributes compete for ~/.gitconfig lock
FS determines FileStore attributes in a background thread and tries to
save the results to the global git configuration. This competed with
LocalDiskRepositoryTestCase#setup trying to save changes to the same
file requiring the same lock. This frequently led to one of the threads
failing to acquire the lock.
Fix this by first initiating determination of FileStore attributes which
then uses a MockSystemReader not using a userConfig stored to disk which
avoids this race for the lock.
Change-Id: I30fcd96bc15100f8ef9b2a9eb3320bb5ace97c67 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Mon, 12 Aug 2019 15:40:39 +0000 (17:40 +0200)]
Improve retry handling when saving FileStoreAttributes fails
- fix handling of interrupts in FileStoreAttributes#saveToConfig
- increase retry wait time to 100ms
- don't wait after last retry
- dont retry if failure is caused by another exception than
LockFailedException
Change-Id: I108c012717d2bcce71f2c6cb9cf0879de704ebc2 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Make supportsAtomicCreateNewFile return true as default
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.
Matthias Sohn [Mon, 12 Aug 2019 10:02:59 +0000 (12:02 +0200)]
Update orbit to R20190602212107-2019-06 to enable backports from master
update
- org.apache.httpcomponents.httpclient to 4.5.6.v20190503-0009
- org.apache.httpcomponents.httpcore to 4.4.10.v20190123-2214
- com.jcraft.jsch" version to 0.1.55.v20190404-1902
- org.mockito to 2.23.0.v20190527-1420
add its dependencies
- net.bytebuddy.byte-buddy 1.9.0.v20181107-1410
- net.bytebuddy.byte-buddy-agent 1.9.0.v20181106-1534
- org.objenesis to 2.6.0.v20180420-1519
FS#getFileStoreAttributes used the real userConfig and not the mocked
one. This led to test errors when running tests with Bazel since it
sandboxes tests which prevents they can write to ~/.gitconfig.
Fix this by first preparing the MockedSystemReader and the mocked config
before calling FS#getFileStoreAttributes.
Also fix ConfigTest which broke due to this change since it inherits
from LocalDiskRepositoryTestCase and calls its setup method which was
changed here. We can no longer assert by comparing plain text since FS
adds FileStoreAttributes to the mocked userConfig. Also the default
options seen by this test changed since we now use a mocked config.
Change-Id: I76bc7c94953fe979266147d3b309a68dda9d4dfe Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Sat, 10 Aug 2019 21:34:58 +0000 (23:34 +0200)]
Ensure we use MockSystemReader in tests
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>
Matthias Sohn [Sat, 10 Aug 2019 21:28:07 +0000 (23:28 +0200)]
Remove FileBasedConfig.load(boolean) introduced in d45219ba
We can't add this method to the super class StoredConfig since that
abstracts from filesystem storage. MockSystemReader.MockConfig is a
StoredConfig and is also used by tests for dfs based storage. Hence
remove this leaky abstraction.
This implies we always use the fallback FileStoreAttributes which means
a config file modification is considered racy within the first 2
seconds. This should not be an issue since typically configs change
rarely and re-reading a config within the racy period is relatively
cheap since configs are small.
Change-Id: Ia2615addc24a7cadf3c566ee842c6f4f07e159a5 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Thu, 8 Aug 2019 10:30:47 +0000 (12:30 +0200)]
Merge branch 'stable-5.2' into stable-5.3
* stable-5.2:
Fix OpenSshConfigTest#config
FileSnapshot: fix bug with timestamp thresholding
In LockFile#waitForStatChange wait in units of file time resolution
Cache FileStoreAttributeCache per directory
Fix FileSnapshot#save(long) and FileSnapshot#save(Instant)
Persist minimal racy threshold and allow manual configuration
Measure minimum racy interval to auto-configure FileSnapshot
Reuse FileUtils to recursively delete files created by tests
Fix FileAttributeCache.toString()
Add test for racy git detection in FileSnapshot
Repeat RefDirectoryTest.testGetRef_DiscoversModifiedLoose 100 times
Fix org.eclipse.jdt.core.prefs of org.eclipse.jgit.junit
Add missing javadoc in org.eclipse.jgit.junit
Enhance RepeatRule to report number of failures at the end
Fix FileSnapshotTests for filesystem with high timestamp resolution
Retry deleting test files in FileBasedConfigTest
Measure filesystem timestamp resolution already in test setup
Refactor FileSnapshotTest to use NIO APIs
Measure stored timestamp resolution instead of time to touch file
Handle CancellationException in FileStoreAttributeCache
Fix FileSnapshot#saveNoConfig
Use Instant for smudge time in DirCache and DirCacheEntry
Use Instant instead of milliseconds for filesystem timestamp handling
Workaround SecurityException in FS#getFsTimestampResolution
Fix NPE in FS$FileStoreAttributeCache.getFsTimestampResolution
FS: ignore AccessDeniedException when measuring timestamp resolution
Add debug trace for FileSnapshot
Use FileChannel.open to touch file and set mtime to now
Persist filesystem timestamp resolution and allow manual configuration
Increase bazel timeout for long running tests
Bazel: Fix lint warning flagged by buildifier
Update bazlets to latest version
Bazel: Add missing dependencies for ArchiveCommandTest
Bazel: Remove FileTreeIteratorWithTimeControl from BUILD file
Add support for nanoseconds and microseconds for Config#getTimeUnit
Optionally measure filesystem timestamp resolution asynchronously
Delete unused FileTreeIteratorWithTimeControl
FileSnapshot#equals: consider UNKNOWN_SIZE
Timeout measuring file timestamp resolution after 2 seconds
Fix RacyGitTests#testRacyGitDetection
Change RacyGitTests to create a racy git situation in a stable way
Deprecate Constants.CHARACTER_ENCODING in favor of
StandardCharsets.UTF_8
Fix non-deterministic hash of archives created by ArchiveCommand
Update Maven plugins ecj, plexus, error-prone
Update Maven plugins and cleanup Maven warnings
Make inner classes static where possible
Fix API problem filters
Change-Id: I238adfd3080a5fed9d64c3c757297da6ea893918 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Thu, 8 Aug 2019 09:18:14 +0000 (11:18 +0200)]
Merge branch 'stable-5.1' into stable-5.2
* stable-5.1:
Fix OpenSshConfigTest#config
FileSnapshot: fix bug with timestamp thresholding
In LockFile#waitForStatChange wait in units of file time resolution
Cache FileStoreAttributeCache per directory
Fix FileSnapshot#save(long) and FileSnapshot#save(Instant)
Persist minimal racy threshold and allow manual configuration
Measure minimum racy interval to auto-configure FileSnapshot
Reuse FileUtils to recursively delete files created by tests
Fix FileAttributeCache.toString()
Add test for racy git detection in FileSnapshot
Repeat RefDirectoryTest.testGetRef_DiscoversModifiedLoose 100 times
Fix org.eclipse.jdt.core.prefs of org.eclipse.jgit.junit
Add missing javadoc in org.eclipse.jgit.junit
Enhance RepeatRule to report number of failures at the end
Fix FileSnapshotTests for filesystem with high timestamp resolution
Retry deleting test files in FileBasedConfigTest
Measure filesystem timestamp resolution already in test setup
Refactor FileSnapshotTest to use NIO APIs
Measure stored timestamp resolution instead of time to touch file
Handle CancellationException in FileStoreAttributeCache
Fix FileSnapshot#saveNoConfig
Use Instant for smudge time in DirCache and DirCacheEntry
Use Instant instead of milliseconds for filesystem timestamp handling
Workaround SecurityException in FS#getFsTimestampResolution
Fix NPE in FS$FileStoreAttributeCache.getFsTimestampResolution
FS: ignore AccessDeniedException when measuring timestamp resolution
Add debug trace for FileSnapshot
Use FileChannel.open to touch file and set mtime to now
Persist filesystem timestamp resolution and allow manual configuration
Increase bazel timeout for long running tests
Bazel: Fix lint warning flagged by buildifier
Update bazlets to latest version
Bazel: Add missing dependencies for ArchiveCommandTest
Bazel: Remove FileTreeIteratorWithTimeControl from BUILD file
Add support for nanoseconds and microseconds for Config#getTimeUnit
Optionally measure filesystem timestamp resolution asynchronously
Delete unused FileTreeIteratorWithTimeControl
FileSnapshot#equals: consider UNKNOWN_SIZE
Timeout measuring file timestamp resolution after 2 seconds
Fix RacyGitTests#testRacyGitDetection
Change RacyGitTests to create a racy git situation in a stable way
Deprecate Constants.CHARACTER_ENCODING in favor of StandardCharsets.UTF_8
Fix non-deterministic hash of archives created by ArchiveCommand
Update Maven plugins ecj, plexus, error-prone
Update Maven plugins and cleanup Maven warnings
Make inner classes static where possible
Fix API problem filters
Change-Id: Ia57385b2a60f48a5317c8d723721c235d7043a84 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Thu, 8 Aug 2019 08:10:12 +0000 (10:10 +0200)]
Fix OpenSshConfigTest#config
- 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>
Increase the safety factor to 2.5x for extra safety if max of measured
timestamp resolution and measured minimal racy threshold is < 100ms, use
1.25 otherwise since for large filesystem resolution values the
influence of finite resolution of the system clock should be negligible.
Before, not yet using the newly introduced minRacyThreshold measurement,
the threshold was 1.1x FS resolution, and we could issue the
following sequence of events,
In this case, the difference between create-file and read is 5ms,
which exceeded the 4ms FS resolution, even though the events together
took just 2ms of runtime.
Reproduce with:
bazel test --runs_per_test=100 \
//org.eclipse.jgit.test:org_eclipse_jgit_internal_storage_file_FileSnapshotTest
The file system timestamp resolution is 4ms in this case.
This code assumes that the kernel and the JVM use the same clock that
is synchronized with the file system clock. This seems plausible,
given the resolution of System.currentTimeMillis() and the latency for
a gettimeofday system call (typically ~1us), but it would be good to
justify this with specifications.
Also cover a source of flakiness: if the test runs under extreme load,
then we could have
start
create-file
<long delay>
read
end
which would register as an unmodified file. Avoid this by skipping the
test if end-start is too big.
[msohn]:
- downported from master to stable-5.1
- skip test if resolution is below 10ms
- adjust safety factor to 1.25 for resolutions above 100ms
Change-Id: I87d2cf035e01c44b7ba8364c410a860aa8e312ef Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Mon, 5 Aug 2019 16:00:35 +0000 (18:00 +0200)]
In LockFile#waitForStatChange wait in units of file time resolution
Since we now measure file time resolution we can use it to replace the
hard coded wait time of 25ms. FileSnapshot#equals will return true until
the mtime of the old (o) and the new FileSnapshot (n) differ by at least
one file time resolution.
Change-Id: Icb713a80ce9eb929242ed083406bfb6650c72223 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Fri, 19 Jul 2019 15:35:27 +0000 (17:35 +0200)]
Cache FileStoreAttributeCache per directory
Cache FileStoreAttributeCache entries since looking up FileStore for a
file may be expensive on some platforms.
Implement a simple LRU cache based on ConcurrentHashMap using a simple
long counter to order access to cache entries.
Change-Id: I4881fa938ad2f17712c05da857838073a2fc4ddb Signed-off-by: Matthias Sohn <matthias.sohn@sap.com> Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com> Also-By: Marc Strapetz <marc.strapetz@syntevo.com>
Matthias Sohn [Wed, 17 Jul 2019 14:31:42 +0000 (16:31 +0200)]
Persist minimal racy threshold and allow manual configuration
To enable persisting the minimal racy threshold per FileStore add a
new config option to the user global git configuration:
- Config section is "filesystem"
- Config subsection is concatenation of
- Java vendor (system property "java.vendor")
- Java version (system property "java.version")
- FileStore's name, on Windows we use the attribute volume:vsn instead
since the name is not necessarily unique.
- separated by '|'
e.g.
"AdoptOpenJDK|1.8.0_212-b03|/dev/disk1s1"
The same prefix is used as for filesystem timestamp resolution, so
both values are stored in the same config section
- The config key for minmal racy threshold is "minRacyThreshold" as a
time value, supported time units are those supported by
DefaultTypedConfigGetter#getTimeUnit
- measure for 3 seconds to limit runtime which depends on hardware, OS
and Java version being used
If the minimal racy threshold is configured for a given FileStore the
configured value is used instead of measuring it.
When the minimal racy threshold was measured it is persisted in the user
global git configuration.
Rename FileStoreAttributeCache to FileStoreAttributes since this class
is now declared public in order to enable exposing all attributes in one
object.
Change-Id: I22195e488453aae8d011b0a8e3276fe3d99deaea Signed-off-by: Matthias Sohn <matthias.sohn@sap.com> Also-By: Marc Strapetz <marc.strapetz@syntevo.com>
Matthias Sohn [Mon, 15 Jul 2019 13:00:09 +0000 (15:00 +0200)]
Measure minimum racy interval to auto-configure FileSnapshot
By running FileSnapshotTest#detectFileModified we found that the sum of
measured filesystem timestamp resolution and measured clock resolution
may yield a too small interval after a file has been modified which we
need to consider racily clean. In our tests we didn't find this behavior
on all systems we tested on, e.g. on MacOS using APFS and Java 8 and 11
this effect was not observed.
On Linux (SLES 15, kernel 4.12.14-150.22-default) we collected the
following test results using Java 8 and 11:
In 23-98% of 10000 test runs (depending on filesystem type and Java
version) the test failed, which means the effective interval which needs
to be considered racily clean after a file was modified is larger than
the measured file timestamp resolution.
"delta" is the observed interval after a file has been modified but
FileSnapshot did not yet detect the modification:
"resolution" is the measured sum of file timestamp resolution and clock
resolution seen in Java.
Java version filesystem failures resolution min delta max delta
1.8.0_212-b04 btrfs 98.6% 1 ms 3.6 ms 6.6 ms
1.8.0_212-b04 ext4 82.6% 3 ms 1.1 ms 4.1 ms
1.8.0_212-b04 xfs 23.8% 4 ms 3.7 ms 3.9 ms
1.8.0_212-b04 zfs 23.1% 3 ms 4.8 ms 5.0 ms
11.0.3+7 btrfs 98.1% 3 us 0.7 ms 4.7 ms
11.0.3+7 ext4 98.1% 6 us 0.7 ms 4.7 ms
11.0.3+7 xfs 98.5% 7 us 0.1 ms 8.0 ms
11.0.3+7 zfs 98.4% 7 us 0.7 ms 5.2 ms
Mac OS
1.8.0_212 APFS 0% 1 s
11.0.3+7 APFS 0% 6 us
The observed delta is not distributed according to a normal gaussian
distribution but rather random in the observed range between "min delta"
and "max delta".
Run this test after measuring file timestamp resolution in
FS.FileAttributeCache to auto-configure JGit since it's unclear what
mechanism is causing this effect.
In FileSnapshot#isRacyClean use the maximum of the measured timestamp
resolution and the measured "delta" as explained above to decide if a
given FileSnapshot is to be considered racily clean. Add a 30% safety
margin to ensure we are on the safe side.
Change-Id: I1c8bb59f6486f174b7bbdc63072777ddbe06694d Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Fri, 12 Jul 2019 07:49:13 +0000 (09:49 +0200)]
Add test for racy git detection in FileSnapshot
Repeat the test 10000 times to get statistics if measured
fsTimestampResolution is working in practice to detect racy git
situations.
Add a class to compute statistics for this test. Log delta between
lastModified and time when FileSnapshot failed to detect modification.
This happens if the racy git limit determined by measuring filesystem
timestamp resolution and clock resolution is too small. If it would be
correct FileSnapshot would always detect modification or mark it
modified if time since modification is smaller than the racy git limit.
Change-Id: Iabe7af1a7211ca58480f8902d4fa4e366932fc77 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Fri, 12 Jul 2019 07:45:09 +0000 (09:45 +0200)]
Enhance RepeatRule to report number of failures at the end
In order to enable counting how frequently a test fails if repeated add
option abortOnFailure. If it is true the test aborts on the first
failure. Otherwise it runs the configured number of repetitions and, if
there was any failure, throws a RepeatException reporting how many of
the test repetitions failed.
Change-Id: Ic47de44d4a6273fddf04b9993ad989903efb40c3 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 10 Jul 2019 14:17:21 +0000 (16:17 +0200)]
Fix FileSnapshotTests for filesystem with high timestamp resolution
When filesystem timestamp resolution is very high some tests don't work
since runtime of the test setup is too long to reach a racily clean
FileSnapshot. Hence skip these tests when timestamp resolution is higher
than 10 millisecond.
Change-Id: Ie47dd10eda22037b5c1ebff6b6becce0654ea807 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Mon, 8 Jul 2019 08:20:16 +0000 (10:20 +0200)]
Measure stored timestamp resolution instead of time to touch file
Measure granularity of timestamps stored in the filesystem by setting
and then getting lastModified timestamp until the read value changed.
Increase increment exponentially to limit number of iterations starting
with 1 microsecond since Java's FileTime (up to Java 12) truncates
timestamps to 1 microsecond resolution. The chosen algorithm yields 2000
steps between 1 ms and 2.5 s.
Also measure clock resolution and add that for the total timestamp
resolution. This avoids systematic measurement errors introduced by
doing IO to touch a file.
Change-Id: I9b37138619422452373e298d9d8c7cb2c384db3f Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Tue, 2 Jul 2019 23:07:14 +0000 (01:07 +0200)]
Use Instant instead of milliseconds for filesystem timestamp handling
This enables higher file timestamp resolution on filesystems like ext4,
Mac APFS (1ns) or NTFS (100ns) providing high timestamp resolution on
filesystem level.
Note:
- on some OSes Java 8,9 truncate milliseconds, see
https://bugs.openjdk.java.net/browse/JDK-8177809, fixed in Java 10
- UnixFileAttributes truncates timestamp resolution to microseconds when
converting the internal representation to FileTime exposed in the API,
see https://bugs.openjdk.java.net/browse/JDK-8181493
- WindowsFileAttributes also provides only microsecond resolution
Change-Id: I25ffff31a3c6f725fc345d4ddc2f26da3b88f6f2 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 26 Jun 2019 07:10:31 +0000 (09:10 +0200)]
FS: ignore AccessDeniedException when measuring timestamp resolution
It seems on cygwin creating a file under a writable directory can fail
with AccessDeniedException. Log a warning in this case and fallback to
worst case timestamp resolution of 2 seconds.
Bug: 548648
Change-Id: Ic50c31ce9dc9ccadd4db5247df929418ac62d45c Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 10 Jul 2019 08:19:01 +0000 (10:19 +0200)]
Add debug trace for FileSnapshot
Checking lastModified is time critical hence debug trace is the only way
to analyze issues since debugging is impractical.
Also add configuration for buffering of log4j output to reduce runtime
impact when debug trace is on. Limit buffer to 1MiB and comment this
configuration out since we may not always want to use buffering.
Change-Id: Ib1a0537b67c8dc3fac994a77b42badd974ce6c97 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Tue, 25 Jun 2019 23:11:12 +0000 (01:11 +0200)]
Use FileChannel.open to touch file and set mtime to now
Use options
- StandardOpenOption.CREATE to create touched file if not existing
- StandardOpenOption.SYNC to enforce synch of data and meta data changes
- StandardOpenOption.WRITE
Also set mtime explicitly in FileUtils#touch to the current system time.
This should fix that the previous implementation didn't work on
- locally cached Windows network share (CSC-CACHE filesystem) mapped as
a drive
- nfsv4 mounts on Linux
and that it didn't create unborn file like Linux command "touch".
Apache common's and Guava's touch() use the same approach.
Immediately after creating the probe file used to measure timestamp
resolution touch it. This ensures we always use the local system clock
when measuring filesystem timestamp resolution. This should prevent that
clock skew could influence the measured timestamp resolution in case of
a mounted network filesystem.
Bug: 548598
Change-Id: Iaeaf5967963f582395a195aa637b8188bfadac60 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Fri, 21 Jun 2019 16:12:14 +0000 (18:12 +0200)]
Persist filesystem timestamp resolution and allow manual configuration
To enable persisting filesystem timestamp resolution per FileStore add a
new config section to the user global git configuration:
- Config section is "filesystem"
- Config subsection is concatenation of
- Java vendor (system property "java.vm.vendor")
- runtime version (system property "java.vm.version")
- FileStore's name
- separated by '|'
e.g.
"AdoptOpenJDK|1.8.0_212-b03|/dev/disk1s1"
The prefix is needed since some Java versions do not expose the full
timestamp resolution of the underlying filesystem. This may also
depend on the underlying operating system hence concrete key values
may not be portable.
- Config key for timestamp resolution is "timestampResolution" as a time
value, supported time units are those supported by
DefaultTypedConfigGetter#getTimeUnit
If timestamp resolution is already configured for a given FileStore
the configured value is used instead of measuring the resolution.
When timestamp resolution was measured it is persisted in the user
global git configuration.
If locking the git config file fails retry saving the resolution up to 5
times in order to workaround races with another thread.
In order to avoid stack overflow use the fallback filesystem timestamp
resolution when loading FileBasedConfig which creates itself a
FileSnapshot to help checking if the config changed.
Note:
- on some OSes Java 8,9 truncate to milliseconds or seconds, see
https://bugs.openjdk.java.net/browse/JDK-8177809, fixed in Java 10
- UnixFileAttributes up to Java 12 truncates timestamp resolution to
microseconds when converting the internal representation to FileTime
exposed in the API, see https://bugs.openjdk.java.net/browse/JDK-8181493
- WindowsFileAttributes also provides only microsecond resolution up to
Java 12
Hence do not attempt to manually configure a higher timestamp resolution
than supported by the Java version being used at runtime.
Bug: 546891
Bug: 548188
Change-Id: Iff91b8f9e6e5e2295e1463f87c8e95edf4abbcf8 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Mon, 10 Jun 2019 21:35:20 +0000 (23:35 +0200)]
Increase bazel timeout for long running tests
EolRepositoryTest and GcCommitSelectionTest timed out frequently when
running unit tests using bazel with the default timeout "moderate"
(300s). Increase timeout of these tests to "long" (900s).
Change-Id: I43588cf950f55b50f868d9fe9c66d22bd428a54c Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
In order to avoid blocking on the main thread during measurement
interactive applications like EGit may want to measure the filesystem
timestamp resolution asynchronously.
In order to enable measurement in the background call
FileStoreAttributeCache.setAsyncfileStoreAttrCache(true)
before the first access to cached FileStore attributes.
Bug: 548188
Change-Id: I8c9a2dbfc3f1d33441edea18b90e36b1dc0156c7 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
In commit I5485db55 ("Fix FileSnapshot's consideration of file size"),
the special casing of UNKNOWN_SIZE was forgotten.
This change, together with I493f3b57b ("Measure file timestamp
resolution used in FileSnapshot") introduced a regression that would
occasionally surface in Gerrit integration tests marked UseLocalDisk,
with the symptom that creating the Admin user in NoteDb failed with a
LOCK_FAILURE.
Matthias Sohn [Fri, 21 Jun 2019 09:00:05 +0000 (11:00 +0200)]
Timeout measuring file timestamp resolution after 2 seconds
It was reported that measuring file timestamp resolution may hang
indefinitely on nfs. Hence timeout this measurement at the known worst
filesystem timestamp resolution (FAT) of 2 seconds.
Bug: 548188
Change-Id: I17004b0aa49d5b0e76360a008af3adb911b289c0 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Tue, 18 Jun 2019 09:32:59 +0000 (11:32 +0200)]
Fix RacyGitTests#testRacyGitDetection
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>
Matthias Sohn [Sun, 16 Jun 2019 21:58:06 +0000 (23:58 +0200)]
Fix non-deterministic hash of archives created by ArchiveCommand
Archives created by the ArchiveCommand didn't produce deterministic
archive hashes. For RevCommits RevWalk.parseTree returns the root tree
instead of the RevCommit hence retrieving the commit's timestamp didn't
work. Instead use RevWalk.parseAny and extract the tree manually.
Archive entries store timestamps with 1 second resolution hence we need
to wait longer when creating the same archive twice and compare archive
hashes. Otherwise hash comparison in tests wouldn't fail without this
patch.
Bug: 548312
Change-Id: I437d515de51cf68265584d28a8446cebe6341b79 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Tue, 18 Jun 2019 14:55:02 +0000 (16:55 +0200)]
Update Maven plugins ecj, plexus, error-prone
update Maven plugins
- ecj to 3.17.0
- error_prone_core to 2.3.3
- plexus-compiler-eclipse to 2.8.5
- plexus-compiler-javac to 2.8.5
- plexus-compiler-javac-errorprone to 2.8.5
Change-Id: I51ecb44538915ed84db041510562394bce977a3e Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Tue, 18 Jun 2019 14:55:02 +0000 (16:55 +0200)]
Update Maven plugins and cleanup Maven warnings
update Maven plugins
- jacoco-maven-plugin to 0.8.4
- japicmp-maven-plugin to 0.14.1
- maven-compiler-plugin to 3.8.1
- maven-deploy-plugin to 3.0.0-M1
- maven-enforcer-plugin to 3.0.0-M2
- maven-install-plugin to 3.0.0-M1
- maven-jar-plugin to 3.1.2
- maven-javadoc-plugin to 3.1.0
- maven-jxr-plugin to 3.0.0
- maven-pmd-plugin to 3.12.0
- maven-resources-plugin to 3.1.0
- maven-shade-plugin to 3.2.1
- maven-source-plugin to 3.1.0
- maven-surefire-plugin to 3.0.0-M3
- spotbugs-maven-plugin to 3.1.12
- tycho to 1.3.0
- tycho-pack200a-plugin to 1.3.0
- tycho-pack200b-plugin to 1.3.0
Cleanup Maven warnings
- pin version of all used Maven plugins
- remove deprecated way to declare minimum Maven version
Change-Id: If23e2e2bb03e5e1e7b1eb9d4924a8faa0aa3704e Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
David Pursehouse [Tue, 25 Sep 2018 05:11:23 +0000 (14:11 +0900)]
Make inner classes static where possible
As reported by Error Prone:
An inner class should be static unless it references members of its
enclosing class. An inner class that is made non-static unnecessarily
uses more memory and does not make the intent of the class clear.
See https://errorprone.info/bugpattern/ClassCanBeStatic
Change-Id: Ib99d120532630dba63cf400cc1c61c318286fc41 Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
(cherry picked from commit ee40efcea44bc0c9a28afe29a80c87636947484e)
Matthias Sohn [Wed, 5 Jun 2019 19:18:21 +0000 (21:18 +0200)]
Merge branch 'stable-5.2' into stable-5.3
* stable-5.2:
Prepare 5.1.9-SNAPSHOT builds
JGit v5.1.8.201906050907-r
Test detecting modified packfiles
Enhance fsTick() to use filesystem timer resolution
Add debug trace to measure time needed to open pack index
Extend FileSnapshot for packfiles to also use checksum to detect changes
Wait opening new packfile until it can't be racy anymore
Avoid null PackConfig in GC
Add FileSnapshot test testing recognition of file size changes
Capture reason for result of FileSnapshot#isModified
Skip FileSnapshotTest#testSimulatePackfileReplacement on Windows
Tune max heap size for tests
Fix FileSnapshotTest.testNewFileNoWait() to match its javadoc
ObjectDirectory: fix closing of obsolete packs
Include filekey file attribute when comparing FileSnapshots
Measure file timestamp resolution used in FileSnapshot
Fix FileSnapshot's consideration of file size
Fix API problem filters
Fix API problem filters
Matthias Sohn [Wed, 5 Jun 2019 13:43:29 +0000 (15:43 +0200)]
Merge branch 'stable-5.1' into stable-5.2
* stable-5.1:
Prepare 5.1.9-SNAPSHOT builds
JGit v5.1.8.201906050907-r
Test detecting modified packfiles
Enhance fsTick() to use filesystem timer resolution
Add debug trace to measure time needed to open pack index
Extend FileSnapshot for packfiles to also use checksum to detect changes
Wait opening new packfile until it can't be racy anymore
Avoid null PackConfig in GC
Add FileSnapshot test testing recognition of file size changes
Capture reason for result of FileSnapshot#isModified
Skip FileSnapshotTest#testSimulatePackfileReplacement on Windows
Tune max heap size for tests
Fix FileSnapshotTest.testNewFileNoWait() to match its javadoc
ObjectDirectory: fix closing of obsolete packs
Include filekey file attribute when comparing FileSnapshots
Measure file timestamp resolution used in FileSnapshot
Fix FileSnapshot's consideration of file size
Fix API problem filters
Change-Id: I3ac77bfa03f7436de12ab86e1bba29afee5ccd01 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Test that JGit detects that packfiles have changed even if they are
repacked multiple times in one tick of the filesystem timer.
Test that this detection works also when repacking doesn't change the
length or the filekey of the packfile. In this case where a modified
file can't be detected by looking at file metadata JGit should still
detect too fast modification by racy git checks and trigger rescanning
the pack list and consequently rereading of packfile content.
Change-Id: I67682cfb807c58afc6de9375224ff7489d6618fb Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Enhance fsTick() to use filesystem timer resolution
RepositoryTestCase.fsTick() was was waiting 64, 128, 256, ... milliseconds
until it detected that the filesystem timer has ticked. Make use of
the filesystemtimer resolution information in FS to sleep a fraction
of the filesystem timer resolution. That raises probability to wake up
shortly after the filesystem timer has ticked.
Matthias Sohn [Sun, 26 May 2019 20:40:47 +0000 (22:40 +0200)]
Extend FileSnapshot for packfiles to also use checksum to detect changes
If the attributes of FileSnapshot don't detect modification of a
packfile read the packfile's checksum and compare it against the
checksum cached in the loaded packfile.
Since reading the checksum needs less IO than reloading the complete
packfile this may help to reduce the overhead to detect modficiation
when a gc completes while ObjectDirectory scans for packfiles in another
thread.
Bug: 546891
Change-Id: I9811b497eb11b8a85ae689081dc5d949ca8c4be5 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Wed, 8 May 2019 23:23:15 +0000 (01:23 +0200)]
Wait opening new packfile until it can't be racy anymore
If
- pack.waitPreventRacyPack = true (default is false)
- packfile size > pack.minSizePreventRacyPack (default is 100 MB)
wait after a new packfile was written and before it is opened until it
cannot be racy anymore.
If a new packfile is accessed while it's still racy at least the pack's
index will be reread by ObjectDirectory.scanPacksImpl(). Hence it may
save resources to wait one tick of the file system timer to avoid this
reloading. On filesystems with a coarse timestamp resolution it may be
beneficial to skip this wait for small packfiles.
Bug: 546891
Change-Id: I0e8bf3d7677a025edd2e397dd2c9134ba59b1a18 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Matthias Sohn [Thu, 9 May 2019 22:58:42 +0000 (00:58 +0200)]
Capture reason for result of FileSnapshot#isModified
This allows to verify the expected behavior in
FileSnapshotTest#testSimulatePackfileReplacement and enables extending
FileSnapshot for packfiles to read the packfile's checksum as another
criterion to detect modifications without reading the full content.
Also add another field capturing the result of the last check if
lastModified was racily clean.
Remove unnecessary determination of raciness in the constructor. It was
determined twice in all relevant cases.
Change-Id: I100a2f49d7949693d7b72daa89437e166f1dc107 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>