PerformanceLogContext threw NullPointerException when multiple threads
tried to add an event to the PerformanceLogContext. The cause for this
is that the ThreadLocal was initialized only in the class constructor
for the first thread; for subsequent threads it was null.
To fix this initialize eventRecords for each thread.
Change-Id: I18ef67dff8f0488e3ad28c9bbc18ce73d5168cf9
Signed-off-by: Alexa Panfil <alexapizza@google.com>
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>
This reverts commit f802f06e7f.
I had misunderstood how protocol V2 works. This implementation only
works if the negotiation during fetch is done in one round.
Fixing this is substantial work in BasePackFetchConnection. Basically
I think I'd have to change back negotiate to the V0 version, and have
a doFetch() that does
if protocol V2
doFetchV2()
else
doFetchV0()
with doFetchV0 the old code, and doFetchV2 completely new.
Plus there would need to be a HTTP test case requiring several
negotiation rounds.
This is a couple of days work at least, and I don't know when I will
have the time to revisit this. So although the rest of the code is
fine I prefer to back this out completely and not leave a only half
working implementation in the code for an indeterminate time.
Bug: 553083
Change-Id: Icbbbb09882b3b83f9897deac4a06d5f8dc99d84e
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Make all transports request protocol V2 when fetching. Depending on
the transport, set the GIT_PROTOCOL environment variable (file and
ssh), pass the Git-Protocol header (http), or set the hidden
"\0version=2\0" (git anon). We'll fall back to V0 if the server
doesn't reply with a version 2 answer.
A user can control which protocol the client requests via the git
config protocol.version; if not set, JGit requests protocol V2 for
fetching. Pushing always uses protocol V0 still.
In the API, there is only a new Transport.openFetch() version that
takes a collection of RefSpecs plus additional patterns to construct
the Ref prefixes for the "ls-refs" command in protocol V2. If none
are given, the server will still advertise all refs, even in protocol
V2.
BasePackConnection.readAdvertisedRefs() handles falling back to
protocol V0. It newly returns true if V0 was used and the advertised
refs were read, and false if V2 is used and an explicit "ls-refs" is
needed. (This can't be done transparently inside readAdvertisedRefs()
because a "stateless RPC" transport like TransportHttp may need to
open a new connection for writing.)
BasePackFetchConnection implements the changes needed for the protocol
V2 "fetch" command (simplified ACK handling, delimiters, section
headers).
In TransportHttp, change readSmartHeaders() to also recognize the
"version 2" packet line as a valid smart server indication.
Adapt tests, and run all the HTTP tests not only with both HTTP
connection factories (JDK and Apache HttpClient) but also with both
protocol V0 and V2. Do the same for the SSH transport tests.
Bug: 553083
Change-Id: Ice9866aa78020f5ca8f397cde84dc224bf5d41b4
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This problem occurred when calling SubmoduleWalk#getModuleName if the
first submodule processed has a name and a path which do not match
SubmoduleWalk#getModuleName returned the module path instead of the
module name. In order to fix this SubmoduleWalk#getModuleName needs to
ensure that the modules config is loaded.
Bug: 565776
Change-Id: I36ce1fbc64c4849f9d8e39864b825c6e28d344f8
Signed-off-by: John Dallaway <john@dallaway.org.uk>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Add new performance logging to register events of type duration.
The proposed logging is similar to the performance logging
in OS Gerrit https://gerrit-review.googlesource.com/c/gerrit/+/225628:
a global Singleton (LoggingContext in Gerrit) is
collecting the performance logs in a thread-safe events list,
and at the end of the monitored command the list of events is
retrieved and written to a log, after which it is cleared.
What this patch does:
The main component is the Singleton (PerformanceLogContext), which
is used to collect the records (PerformanceLogRecord) in one place
(ThreadLocal eventsList) from anywhere using
PerformanceLogContext.getInstance().addEvent().
Reason why this change is needed:
The current monitoring in JGit has several issues:
1. git fetch and git push events are handled separately
(PackStatistics and ReceivedPackStatistics), with no unified way
of writing or reading the statistics.
2. PostUploadHook is only invoked on the event of sending the
pack, which means that the PackStatistics is not available for
the fetch requests that did not end with sending the pack
(negotiation requests).
3. The way the logs are created is different from the performance
log approach, so the long-running operations need to be collected
from both performance log (for JGit DFS overridden operations and
Gerrit operations) and gitlog (for JGit ones).
The proposed performance logging is going to solve the above
mentioned issues: it collects all of the performance logs in one
place, thus accounting for the commands that do not result in
sending a pack. The logs are compatible with the ones on Gerrit.
Moreover, the Singleton is accessible anywhere in the call stack,
which proved to be successful in other projects like Dapper
(https://research.google/pubs/pub36356/).
Signed-off-by: Alexa Panfil <alexapizza@google.com>
Change-Id: Iabfe667a3412d8a9db94aabb0f39b57f43469c41
This enables jgit to use any refs in the refs/ namespace when describing
commits.
Signed-off-by: Jason Yeo <jasonyeo88@gmail.com>
Change-Id: I1fa22d1c39c0e2f5e4c2938c9751d8556494ac26
Reason why this change is needed:
Currently the durations of fetch events are computed by
registering time instants with System.currentTimeMillis()
and calculating the differences with simple minus operation,
but multiple sources suggest that the best practice is to use
the Java 8 Duration and Instant objects.
What this patch does:
Get time measurements with Instant.now() instead of
System.currentTimeMillis() and calculate the duration of fetch
events (Reachability checks and Negotiation) using
Duration.between().toMillis().
Signed-off-by: Alexa Panfil <alexapizza@google.com>
Change-Id: I573a0a0562070083cf5a5a196d9710f69a7fa587
Building with Bazel is failing with this error message:
HttpSupport.java:480: error: [OperatorPrecedence] Use grouping
parenthesis to make the operator precedence explicit
if (c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z') {
^
(see https://errorprone.info/bugpattern/OperatorPrecedence)
Did you mean 'if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')) {'?
Change-Id: I96089d3158d06ba981cfdf2b03865261b328de23
Validate the extra headers and log but otherwise ignore invalid
headers. An empty http.extraHeader starts the list afresh.
The http.userAgent is restricted to printable 7-bit ASCII, other
characters are replaced by '.'.
Moves a support method from the ssh.apache bundle to HttpSupport in
the main JGit bundle.
Bug:541500
Change-Id: Id2d8df12914e2cdbd936ff00dc824d8f871bd580
Signed-off-by: James Wynn <james@jameswynn.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This enables EGit to override with a lenient variant that logs the
problem and continues with the default value. EGit needs this because
otherwise a corrupt core.excludesFile entry can render the whole UI
unusable (until the git config is fixed) because any use of the
WorkingTreeIterator will throw an InvalidPathException.
This is not a problem on OS X, where all characters are allowed in
file names. But on Windows some characters are forbidden... see bug
567296. The message of the InvalidPathException is not helpful since
it doesn't point to the origin of the problem. EGit can log a much
better message indicating the offending config file and entry in the
Eclipse error log, where the user can see it.
Bug: 567309
Change-Id: I4e57afa715ff3aaa52cd04b5733f69e53af5b1e0
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Reason why this change is needed:
Getting this metric will help estimate how much time will be saved once
the reachability checks get optimized
What this patch does:
Measure time spent by requestValidator.checkWants() in parseWants() and save
it in an instance of PackStatistics.Accumulator.
Signed-off-by: Alexa Panfil <alexapizza@google.com>
Change-Id: Id7fe4016f96549d9511a2c24052dad93cfbb31a4
Reason why this change is needed:
Getting this metric will help estimate how much time is spent
on negotiation in fetch V2.
What this patch does:
Measure time spent on negotiation rounds in UploadPack.fetchV2()
and save it in an instance of PackStatistics.Accumulator.
This is the same way the statistics are already gathered for
protocol V0/V1.
Change-Id: I14e55dd6ff743cb0b21b4953b533269ef069abb1
When comparing git directory paths to check whether one is a prefix
of another, one must add a slash to avoid false prefix matches when
one directory name is a prefix of another. The path "audio" is not
a prefix of the path "audio-new", but would be a prefix of a path
"audio/new".
Bug: 566799
Change-Id: I6f671ca043c7c2c6044eb05a71dc8cca8d0ee040
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This is useful to access git repositories behind a bastion server
(jump host).
Add a constant for the config; rewrite the whole connection initiation
to parse the value and (recursively) set up the chain of hops. Add
tests for a single hop and two different ways to configure a two-hop
chain.
The connection timeout applies to each hop in the chain individually.
Change-Id: Idd25af95aa2ec5367404587e4e530b0663c03665
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Since there is no negotiation for a push, the client is probably sending
redundant objects and bytes which already exist in the server.
Add more metrics in the stats to quantify it. Duplicated size and number
to measure the size and the number of duplicated objects which should
not be pushed.
Change-Id: Iaacd4761ee9366a0a7ec4e26c508eff45c8744de
Signed-off-by: Yunjie Li <yunjieli@google.com>
* stable-5.9:
Prepare 5.9.1-SNAPSHOT builds
JGit v5.9.0.202009080501-r
[releng] Enable japicmp for the fragments added in 5.8.0
GitlinkMergeTest: fix boxing warnings
Remove unused API problem filters
Add missing since tag on BundleWriter#addObjectsAsIs
GPG: include signer's user ID in the signature
Change-Id: Iaa96f9228752540f446fc232a49f31a738fd8d30
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
* master:
SshdSession: close channel gracefully
jgit: Add DfsBundleWriter
Prepare 5.10.0-SNAPSHOT builds
ResolveMerger: do not content-merge gitlinks on del/mod conflicts
ResolveMerger: Adding test cases for GITLINK deletion
ResolveMerger: choose OURS on gitlink when ignoreConflicts
ResolveMerger: improving content merge readability
ResolveMerger: extracting createGitLinksMergeResult method
ResolveMerger: Adding test cases for GITLINK merge
Back out the version change to 5.10.0-SNAPSHOT which was done on master
already.
Change-Id: I1a6b1f0b8f5773be47823d74f593d13b16a601d5
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>
* changes:
ResolveMerger: do not content-merge gitlinks on del/mod conflicts
ResolveMerger: Adding test cases for GITLINK deletion
ResolveMerger: choose OURS on gitlink when ignoreConflicts
ResolveMerger: improving content merge readability
ResolveMerger: extracting createGitLinksMergeResult method
ResolveMerger: Adding test cases for GITLINK merge
* stable-5.9:
Upgrade maven-resources-plugin to 3.2.0
Upgrade plexus-compiler version to 2.8.8
[bazel] Add missing dependency to slf4j-api
[errorprone] DirCacheEntry: make clear operator precedence
[errorprone] PackWriter#parallelDeltaSearch: avoid suppressed exception
[errorprone] Declare DirCache#version final
Add jgit-4.17-staging target platform for 2020-09
Update target platform to R20200831200620
Signed-off-by: Marco Miller <marco.miller@ericsson.com>
Change-Id: I2e2f41cf6ebbcb45b8978b519db3f1c8f7afb5f4
Previously ResolveMerger tried to make a fulltext merge entry in case
one of sides got deleted regardless of file mode. This is not
applicable for GITLINK type of entry. After this change it is
rendering appropriate merge result.
Signed-off-by: Demetr Starshov <dstarshov@google.com>
Change-Id: Ibdb4557bf8781bdb48bcee6529e37dc80582ed7e
Option ignoreConflicts is used when a caller want to create a virtual
commit and use it in a future merge (recursive merge) or show it on
UI (e.g. Gerrit). According to contract in case of ignoreConflicts
ResolveMerger should populate only stage 0 for entries with merge
conflicts as if there is no conflict. Current implementation breaks
this contract for cases when gitlink revision is ambiguous.
Therefore, always select 'ours' when we merge in ignoreConflicts mode.
This will satisfy the contract contract, so recursive merge can
succeed, however it is an arbitrary decision, so it is not guaranteed
to select best GITLINK in all cases.
GITLINK merging is a special case of recursive merge because of
limitations of GITLINK type of entry. It can't contain more than 1 sha-1
so jgit can't write merge conflicts in place like it can with a blob.
Ideally we could signal the conflict with a special value (like
'0000...'), but that must be supported by all tooling (git fsck, c-git)."
Signed-off-by: Demetr Starshov <dstarshov@google.com>
Change-Id: Id4e9bebc8e828f7a1ef9f83259159137df477d89
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>
Previous code used a minimum granularity of 1 microsecond and would
iterate 233 times on a system where the resolution is 1 second (for
instance, Java 8 on Mac APFS).
New code uses a binary search between the maximum we care about (2
seconds) and zero, with a minimum granularity of also 1 microsecond.
This takes at most 19 iterations (guaranteed). For a file system with 1
second resolution, it takes 4 iterations (1s, 0.5s, 0.8s, 0.9s). With
an up-front check at 1 microsecond and at 1 millisecond this performs
equally well as the old code on file systems with a fine resolution.
(For instance, Java 11 on Mac APFS.)
Also handle obscure cases where the file timestamp implementation may
yield bogus values (as observed on HP NonStop). If such an error case
occurs, log a warning and abort the measurement at the last good value.
Bug: 565707
Change-Id: I82a96729b50c284be7c23fbdf3d0df1bddf60e41
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
LockFile.lock() will create it anyway when the config file is created.
Bug: 565637
Change-Id: I078b89a695193fd76f130f6de7ac1cf26d2f8f0f
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Otherwise locking problems may ensue if the JGit config itself is
on a different file system. Since the internal is already updated,
it is not really important when exactly the value gets persisted.
By queueing up separate Runnables executed by a single thread we
avoid concurrent write access to the JGit config, and nested calls
to getFileStoreAttributes(Path) result in serialized attempts to
write.
The thread for writing the config must not be a daemon thread. If
it were, JVM shutdown might kill it anytime, which may lead to
the config not being written, or worse, a config.lock file being
left behind.
Bug: 566170
Change-Id: I07e3d4c5e029d3cec9ab5895002fc4e0c7948c40
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Alexa Panfil