| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Change 270450 caused the blanking of the Lucene document
upon reindexing if any field caused a StorageException.
Whilst the overall intention was good, the implementation
caused the Lucene index replace operation to continue with
a Document without any fields instead of making the whole
operation fail.
StorageExceptions are thrown when the underlying storage,
being a filesystem or anything else, returns some errors.
Whether the failure is permanent or temporary (e.g. concurrent
GCs, repacking or pruning may cause sporadic StorageException(s))
returning a blank Lucene document was incorrect because, instead
of failing the operation, it was causing the change entry to be
completely removed from the index.
Let the StorageException fail the indexing operation, so that
existing entries may continue to exist, allowing the caller
to retry the operation.
The previous implementation, returning an empty Document, did
not allow any retry, because once the change entry was removed
from the index, it could not be discovered and reindexed anymore
for example by a `gerrit index changes <changenum>`.
Tested manually, applying a randomic StorageException thrown
during the fetching of the ChangeField extensions:
public static Set<String> getExtensions(ChangeData cd) {
if (new Random().nextBoolean()) {
throw new StorageException("Simulated storage exception");
}
return extensions(cd).collect(toSet());
}
Before this change, every time one change indexing throws a
StorageException, it disappears from the index. Eventually,
all changes will be eliminated and only an off-line reindex
or the reindex of all changes in the project would allow to
recover them.
After this change, some of the indexing operations are
successful and other fails. However, retrying the reindexing
operation would allow to reindex all of them.
Even if the above test case looks strange at first sight,
it simulates a real-life scenario where a low percentage
of indexing operation (0.1%) may fail because of sporadic
StorageExceptions. Before this change, some index entries
were missing on a daily basis (5 to 10 changes per day),
whilst after this change all indexing operation can be
retried and do not result in any indexing entry loss.
Bug: Issue 314113030
Release-Notes: Fail the change reindex operation upon StorageException(s)
Change-Id: Ia121f47f7a68c290849a22dea657804743a26b0d
|
|
|
|
|
|
|
|
|
|
|
| |
These calls used to fail silently and are of no further use.
This change is part of a series that allows for the fake change index
to run in integration tests. The fake index throws an exception in
case reindexing fails, which made this test fail.
Release-Notes: skip
Change-Id: I9c9d3f7613bca057bf0963442077bb6c0b3dab51
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
receivePackOmitsMissingObject wants to test a scenario where we
have a change ref, but the patch set ref / commit was deleted or
is otherwise gone.
It did that by adding a new patch set pointing to a non-existent
SHA1 and triggered a reindex of that change. However, that reindex
fails silently because we submit the reindex task to another
executor and provide no feedback to the caller.
This commit modifies the test and just deletes the patch set ref
which makes for the same scenario but without the reindex trouble.
This change is part of a series that allows for the fake change index
to run in integration tests. The fake index throws an exception in
case reindexing fails, which made this test fail.
Release-Notes: skip
Change-Id: Icf79795a343a6abca28c6504b279eb0a5c84fad2
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Jetty 9.4.53.v20231009 includes the following two security fixes:
- CVE-2023-36478 [1] - zero-days security issue discovered on
the 10th of October, also known as "HTTP/2 Rapid Reset"
- CVE-2023-44487 [2] - HTTP/2 Stream Cancellation Attack
[1] https://nvd.nist.gov/vuln/detail/CVE-2023-36478
[2] https://nvd.nist.gov/vuln/detail/CVE-2023-44487
Release-Notes: Update Jetty to 9.4.53.v20231009 with critical security fixes
Change-Id: Ie93fbcb8b35d9e4997dc0578893a8856b56b173c
(cherry picked from commit c49057e05d35ff2ad1a7307aa9168b84ae7588db)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since the introduction of Caffeine as alternative to Guava
in Change 244612, the execution of the cache event listeners moved
to a background thread, run by the ForkJoinPool's common pool [1].
The subtle difference has caused issues to the plugins that are
registering listeners, like the high-availability and multi-site:
the consequences have been quite serious because of the inability
to understand if the eviction was caused by a forwarded cache
eviction event or by an execution of Gerrit API or other internals
that caused the removal of the entry.
The use of the JVM common pool has several disadvantages and, under
certain conditions [2], it may even lead to deadlocks or unexpected
blockages.
By introducing the cache.threads option, decouple the cache background
threads execution and allow to configure an explicit separate
thread pool which can be tuned and decoupled from the rest of the
JVM common threads.
Also, allow to restore the plugins' cache listeners legacy behaviour
without losing the ability to leverage the performance of Caffeine
cache vs. the traditional Guava.
By default, this change is a NO-OP because it preserves the current
behaviour of background execution tasks of the Caffeine cache.
Introduce DefaultMemoryCacheFactoryTest class from stable-3.4 for
avoiding further conflicts when merging upstream.
References:
[1] https://github.com/ben-manes/caffeine/wiki/Guava#asynchronous-notifications
[2] https://dzone.com/articles/be-aware-of-forkjoinpoolcommonpool
Release-Notes: introduce cache.threads option to allow custom executors for Caffeine caches
Bug: Issue 16565
Change-Id: I204abd1bdbf2bbed5b3d982d14cbc5549ac96ace
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If9da362140 introduced regression in project root computation.
Also add test coverage for Gitweb servlet so that a similar regresssion
could be avoided in the future.
Test Plan:
bazel test javatests/com/google/gerrit/httpd/...
Bug: Issue 16449
Release-Notes: Fix project root computation in Gitweb servlet
Change-Id: Ia1a7bdea55db4deb55a3b82a292890e7febbe675
|
|
|
|
|
|
|
|
|
| |
Printing cache statistics can be slow, particularly when the
caches are large. For example, when the caches are ~400G,
printing cache stats takes over 30 mins.
Release-Notes: Printing cache stats is disabled by default for Init and Reindex
Change-Id: I848d6dfef0be9dee9ebac8a597dd7e617cbe30e4
|
|\
| |
| |
| |
| |
| |
| |
| |
| | |
* stable-3.1:
Predicate: Add safety check against not(any())
Add an index-safe predicate that matches all changes
Release-Notes: skip
Change-Id: I56708f2d9a3218f2981e65b867003d0ea714407a
|
| |\
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
* stable-3.0:
Predicate: Add safety check against not(any())
Add an index-safe predicate that matches all changes
Release-Notes: skip
Change-Id: Id37339b406675721a4bac3411c6e2874986954bb
|
| | |\
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-2.16:
Predicate: Add safety check against not(any())
Add an index-safe predicate that matches all changes
Release-Notes: skip
Change-Id: Ia64e074194cd561bc55a6f0e7976633aaa9bb2ff
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
It's tempting to use as a way to say "match no results", but it's very
unsafe. This simple check won't catch all misuses, but it would have
prevented Iad564dd47 from causing production problems.
Fix one obvious caller that now trips this check. This was a real
problem: a bare [is:watched] query on a large site for a user with no
project watches required iterating all changes to return an empty
result. We have real instances of this causing extremely long requests
in our server logs.
Release-Notes: skip
Change-Id: Ib43f7466db5db315bf87558a37f18a1832ec641f
(cherry picked from commit 02bab2161cd7ce7f682a28dad496a4465cb40844)
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Iad564dd47 was problematic in the error case because it returned
Predicate.not(Predicate.any()). The "any" predicate is not an index
predicate, which produces at least two problems:
* When used in a with another non-index predicate, it
might throw "no ChangeDataSource" from AndSource/OrSource.
* More dangerously, when combined with an index predicate, it is used as
a post-filtering predicate. A post-filtering predicate that matches no
results means it gets applied to every change in the index before
returning an empty set, which is bad.
Instead, reuse a handy existing index predicate that is designed to
return no results efficiently from the index:
ChangeStatusPredicate.NONE. Expose this from a more general location,
ChangeIndexPredicate#none().
Release-Notes: skip
Change-Id: Ic9a7e277070cff7768ec91c1972cf8e9f6deacb1
(cherry picked from commit d9679cdb605b8a10977e1fc821f57dd5749b1dd5)
|
|\| | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.1:
Cache repository locations in LocalDiskRepositoryManager
Release-Notes: skip
Change-Id: I5443e5174552f2a8369f5d97162b9cb24cd2ae26
|
| |\| |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.0:
Cache repository locations in LocalDiskRepositoryManager
Release-Notes: skip
Change-Id: I5b6cd9e19c4a0b052da9705e0a8ddb0eced148c2
|
| | |\|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-2.16:
Cache repository locations in LocalDiskRepositoryManager
Release-Notes: skip
Change-Id: I39713079d081e7b80f59dc36e6181f4daa565c50
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Obtaining the actual location of a repository using base-path
and project name can be slow as it involves some guessing to
locate the repository. Cache the locations once they are
obtained to avoid repeated work, thereby improving performance
when opening repositories.
For example, on a site with 20k repositories on NFS, ls-projects
with this change takes ~60s and ~90s without. Also, a query which
wraps a large (~2k) list of manifest[1] operators will take ~300ms
with this change and ~2s without it.
[1] https://gerrit.googlesource.com/plugins/manifest
Release-Notes: skip
Change-Id: I8eab3c813c4ac9433e93c7ace96d38efe332be27
|
|\| | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.1:
Limit the number of changes that can be submitted together
Release-Notes: skip
Change-Id: Iab4d9bbbb44cb4e5e3db084f4350e1079f0e7195
|
| |\| |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.0:
Limit the number of changes that can be submitted together
Release-Notes: skip
Change-Id: I63765e81c80fa3bfb661c11ada5b7013f93f93e1
|
| | |\|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-2.16:
Limit the number of changes that can be submitted together
Release-Notes: skip
Change-Id: I263d636ec38f043ad5f6f8157ea5a57e12e7b145
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
When chaining changes together, the sequence of commits to navigate
was previously unbound, causing the potential explosion to millions
of changes.
The explosion could have also been accidental and caused by the push
of a change with a non-existent branch, which would have resulted
in the full scan of the repository for changes.
Introduce a new Gerrit configuration change.maxSubmittableAtOnce with
a safe default of 32767, which would allow any use case that would have
also worked before this change. Navigating over 32767 changes up to
potentially a huge number of commits would have generated a significant
CPU and memory overload and still not resulted in a submittable
chain of changes anyway.
Release-Notes: Limit the number of changes that can be submitted at once
Bug: Issue 16322
Change-Id: Id71aed2341f72708778395359bb6e4d4c270401c
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Instead of retrieving git base path from repository manager, open the
repository and get it directly from its git directory path.
Also, redirect the delegate repository wrapper to the underlying
implementation, allowing to use GitWeb in combination with the
multi-site plugin and the cached-refdb.
The advantage of doing it: we don't need to cast GitRepositoryManager to
the LocalDiskRepositoryManager, so that other implementations would also
work.
Release-Notes: allow using GitWeb with multi-site and cached-refdb
Change-Id: If9da36214063d73953677473082cd16f8f95163a
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
The delegate() method was initially introduced in I8a862ac852
to allow access to the underlying repository while running GC
for sites using multi-site plugin.
The method was initially package private since the callers
where in the same package.
Changing visibility of delegate() method to public
to allow plugins to access it.
Accessing the wrapped repository would otherwise require hacky
workarounds like this Ia3fbdc7e96.
Bug: Issue 15997
Release-Notes: Make DelegateRepository#delegate() method public
Change-Id: I4c8b4f97f9995a7adeca5b6f763e7913c9e8a5e5
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
When GarbageCollectCommand performs GC it expects that instance of the
repository that is passed to it is either FileRepository or
DfsRepository and throws UnsupportedOperationException otherwise.
Detect if DelegateRepository is passed as parameter and make sure that
check is performed on a delegated repository instead.
Bug: Issue 14945
Release-Notes: skip
Change-Id: I8a862ac852f5f98c09662a41234d40c26e944804
(cherry picked from commit a02af610c8ec879c79259a5d4b7a185a22311cd1)
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
By allowing plugins to reuse DelegateRepository functionality
multi-site and high-availability plugins can avoid code
duplication and significantly reduce the code complexity.
Also, make the annotation @UsedAt repeatable for associating
the DelegateRepository functionality with the usage from multiple
plugins.
Bug: Issue 13429
Release-Notes: skip
Change-Id: I2b5f5b215395fc1ef2a8008a71f5c09278d1278b
(cherry picked from commit ed54a267dafeed829722b340ccaee2bbbc7f94b3)
|
|\| | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.1:
Allow async receive-commits to have a thread-local cache
Fix RepoRefCache stale checks during NoteDb rebuild
Lazy load change notes when submit by push
Change-Id: Idb781e53343f98c898ecd4cd1033160b4ce4dfae
Release-Notes: skip
|
| |\| |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.0:
Allow async receive-commits to have a thread-local cache
Fix RepoRefCache stale checks during NoteDb rebuild
Lazy load change notes when submit by push
Change-Id: Ie82487167fc84ec543083b018c3fde1b0a041e1d
Release-Notes: skip
|
| | |\|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-2.16:
Allow async receive-commits to have a thread-local cache
Fix RepoRefCache stale checks during NoteDb rebuild
Lazy load change notes when submit by push
Change-Id: I52fd7c9320ce57488ab54f8b5b24a8eabb002ea5
Forward-Compatible: checked
Release-Notes: skip
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Git receive-commits are executed in a background thread of the
ReceiveCommits pool. The thread-local cache allocated on the
client caller thread isn't used in the execution of the command
making multiple operations (e.g. ACL evaluation, events propagation)
slower because of the missed caching activity.
Release-Notes: Improve caching when merging changes through git push
Change-Id: I34a6e1485294f3156c7f35261fedfc280af1ed43
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
When rebuilding NoteDb meta-data from ReviewDb we are
actually mutating the /meta ref during a reindexing
operation.
This is an edge case that happens only once during
the on-line NoteDb upgrade and the trial mode.
Adapt the RepoRefCache use during NoteDb reindexing
and manage the situation where a staleness check
applies to a ref that has disappeared on disk.
Bug: Issue 15961
Release-Notes: skip
Change-Id: Iac642eb7a9b24882a0c77dbaee24853fb0b29827
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Lazy load the change notes associated with the change to merge
when one or more changes are submitted by a git push of
the change onto its target branch.
When a single change is merged through git push, we need to find
which one is the corresponding change meta to be checked and
merged. However, the building of the entire set of open changes
against the target branch, indexed by their change key was having
the side-effect of loading eagerly changes notes from their /meta ref.
Loading all changes notes can be very slow, O(mins), on
repositories with a large number of refs on slow storage and, also,
it is completely unneeded in this use case.
Loading all the change notes for all open changes had increased the
time to submit a change from a few seconds (via the GUI) to a
few minutes (via git push).
Release-Notes: Performance improvement of change submit via push
Change-Id: I646bc7b13f3359816b6be6354e15ddb5412bfa38
|
|\| | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.1:
Set PerThreadCache as readonly after creating a new patch-set
Set PerThreadCache as readonly when formatting change e-mails
Set PerThreadCache as readonly when formatting change JSON
Set PerThreadCache as readonly after deleting a change
Set PerThreadCache as readonly after abandoning a change
Set PerThreadCache as readonly after merging a change
Set PerThreadCache as readonly after posting review comments
Introduce unloaders on PerThreadCache entries
[I] RepoRefCache: Hold a reference to the refDatabase with ref counting
Remove use of RefCache in ChangeNotes
Cache change /meta ref SHA1 for each change indexing task
Only the commit prefixed by [I] is included, all the others
are reverted in the merge because they are not needed to be merged
upstream from stable-3.2 onwards.
Since stable-3.2 we have more general solution to the problem
with modules/cached-refdb [1] which provides a pluggable cached refdatabase
without the need to fiddle with the thread-local caching.
[1] https://gerrit-review.googlesource.com/admin/repos/modules/cached-refdb,branches
Release-Notes: skip
Change-Id: Ibc0485dfb37e6d4c7c46e34959f9ab6513838ecb
|
| |\| |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.0:
Set PerThreadCache as readonly after creating a new patch-set
Set PerThreadCache as readonly when formatting change e-mails
Set PerThreadCache as readonly when formatting change JSON
Set PerThreadCache as readonly after deleting a change
Set PerThreadCache as readonly after abandoning a change
Set PerThreadCache as readonly after merging a change
Set PerThreadCache as readonly after posting review comments
Introduce unloaders on PerThreadCache entries
RepoRefCache: Hold a reference to the refDatabase with ref counting
Remove use of RefCache in ChangeNotes
Cache change /meta ref SHA1 for each change indexing task
Also adapt TestRepositoryWithRefCounting by overriding the
getIdentifier() method to adapt to the new Repository interface
definition in JGit.
Release-Notes: skip
Change-Id: Ic7a9772f1624513607ed9a255591466763851490
|
| | |\|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-2.16:
Set PerThreadCache as readonly after creating a new patch-set
Set PerThreadCache as readonly when formatting change e-mails
Set PerThreadCache as readonly when formatting change JSON
Set PerThreadCache as readonly after deleting a change
Set PerThreadCache as readonly after abandoning a change
Set PerThreadCache as readonly after merging a change
Set PerThreadCache as readonly after posting review comments
Introduce unloaders on PerThreadCache entries
RepoRefCache: Hold a reference to the refDatabase with ref counting
Remove use of RefCache in ChangeNotes
Cache change /meta ref SHA1 for each change indexing task
Also fix formatting of:
java/com/google/gerrit/acceptance/GerritServer.java
java/com/google/gerrit/server/mail/send/OutgoingEmail.java
Release-Notes: skip
Change-Id: I7fab22d2bfae52294b0f7d237a0d26b9fafa54d6
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Apply the same optimisation made for Change-Id: I1b71bfcf to
the production of events after creating a new patch-set and for returning
the merged change JSON response.
The same considerations of performance improvement and reduction
of /meta refs lookup are valid in this use-case.
Release-Notes: enable the read-only cache of /meta refs for new patch-sets
Change-Id: I58abd12f3779e0a57c1f8cda618905c9c0bb9b7e
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Apply the same optimisation made for Change-Id: I1b71bfcf to
the formatting the change e-mails body, which is done
synchronously to the incoming REST API and thus impacts
the latency perceived by the end user.
The composition of the e-mail is using multiple information
coming from multiple fields from NoteDb and calls the /meta
refs lookups multiple times for the same change.
The same considerations of performance improvement and reduction
of /meta refs lookup are valid in this use-case.
Release-Notes: enable the read-only cache of /meta when producing JSON
Change-Id: I993d4ddc37cf27b66ea9d7323963e761564603fa
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Apply the same optimisation made for Change-Id: I1b71bfcf to
the formatting of change JSON output as return value for REST APIs.
The ChangeJson class needs to read multiple fields from NoteDb
and calls the /meta refs lookups multiple times for the same
change JSON output (e.g. formatting the output JSON of an
abandoned change generates 23 refs lookups calls for a single
change).
The same considerations of performance improvement and reduction
of /meta refs lookup are valid in this use-case.
Release-Notes: enable the read-only cache of /meta when producing JSON
Change-Id: Ic0e747352dcd8a0c2c1348a9878a5a4165777abd
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Apply the same optimisation made for Change-Id: I1b71bfcf to
the production of events after deleting a change.
The same considerations of performance improvement and reduction
of /meta refs lookup are valid in this use-case.
Release-Notes: enable the read-only cache of /meta refs for deleted changes
Change-Id: Ic04e9257b7801bd331390679e00e1f980a9d1d19
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Apply the same optimisation made for Change-Id: I1b71bfcf to
the production of events after abandoning a change.
The same considerations of performance improvement and reduction
of /meta refs lookup are valid in this use-case.
Release-Notes: enable the read-only cache of /meta refs for abandoned changes
Change-Id: Iab71eca5ead78f6462fd3aaf41eb9590fce4684e
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Apply the same optimisation made for Change-Id: I1b71bfcf to
the production of events after merging a change and for returning
the merged change JSON response.
The same considerations of performance improvement and reduction
of /meta refs lookup are valid in this use-case.
Release-Notes: enable the read-only cache of /meta refs for merged changes
Change-Id: I34e147d644dfcdb5a6b1baa32cd4347805e506fc
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Once the review comments mutations have been completed, it is safe to
cache the results of the /meta refs lookups, avoiding the slowdown of
the repeated refs lookups performed during the creation of the event
with the added review and the dispatching of the stream event associated.
The refs lookups could be as slow as 400 ms per lookup for a 500k refs
repository on a shared NFS volume: using a thread-local cache for
/meta ref lookup has a significant improvement (50% less refs lookups)
on the overall user-experience.
Introduce also the safety-net of checking for stale cached refs
against the underlying repository, so that errors can arise
during testing. The staleness checker can be enabled via system
property.
NOTE: The execution of the listeners logic associated with the review
is left outside the read-only request window because of the risk of
a plugin or hook updating the underlying /meta ref and causing a
stale read.
Also rename the current hasReadonlyRequest method to isReadonlyRequest,
which is consistent with the new setter introduced in the PerThreadCache
class.
P.S. DO REVERT this change from stable-3.2 onwards because the
refs lookups have been optimised already with the introduction of the
cached-refdb libModule [1] which is enough to cover all use-cases
introduced in the PerThreadCache for refs lookups.
[1] https://gerrit.googlesource.com/modules/cached-refdb/
Bug: Issue 15798
Release-Notes: enable the read-only cache of /meta refs after review
Change-Id: I1b71bfcff081e66eed897d41539d2be675f34af4
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
RepoRefCache holds reference to a Repository object
which would need once the cache entry is removed.
Add the logic to cleanup the entry resource when the
cache is closed and therefore entries need to be
cleaned up before being removed from cache.
Release-Notes: skip
Change-Id: Id201d4a93792b27d2e1ab8214aa6377387eb567f
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
The RepoRefCache was holding a reference to a refDatabase associated
to a Repository object that was created externally and could have been
already closed once the RepoRefCache.get was called.
Increment and decrement the reference counting for the Repository
object held in the RepoRefCache, making sure that all accesses to
the get() and other methods are always using an active and open
repository.
The problem existed for years but it was never noticed because
of JGit being very flexible on using repositories even when
their reference counting is zero and they are effectively closed.
However, using a closed repository is inconsistent with its interface
and may lead to further issues in the past, because of JGit not
being aware of the repository is actually used by an in-memory
cache holding a reference to it.
Add one extra relevant tests associated with this corner case for
showing the issue, using a TestRepositoryWithRefCounting wrapper
that fails when accessing a refDatabase of a closed repository.
Release-Notes: skip
Change-Id: I4b2c43ea430a0506c73101bc4f6bc62926d5a094
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
The RefCache instance stored in the ChangeNotes refs field
was passed as a nullable parameter but, effectively, always passed
as null value in all invocations form the code-base.
The refs field was also final, which means that it wasn't possible
to memoize or cache any RefCache instance on it anyway.
Remove the field and its use across the code-base, so that the
code becomes easier to read and maintain.
The cache was initially introduced in ChangeNotes with I5e02032d6
and used by the ChangeRebuilderImpl; however the implementation on
how changes are rebuilt was changed with Icc942dba25 which did not
require anymore the use of a RepoRefCache in ChangeNotes. However,
Dave forgot to remove the refs from ChangeNotes which was left unused
since then.
Release-Notes: skip
Change-Id: I729ad8738fd4ceebe72e61f086631f2d109a817a
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Interactive indexing tasks of changes are executed synchronously
with the incoming API: multiple lookups of change /meta
ref name to SHA1 during the reindexing can compromise the
performance of the API. The refs lookups could be as slow
as 400 ms per lookup for a 500k refs repository on a shared
NFS volume.
Leverage the existing RepoRefCache associated with the PerThreadCache
by creating a read-only cache during the execution of the indexing
task.
NOTE: The indexing task may be part of a mutable API
(e.g. review a change); however, the operation itself is readonly
because it is executed once the mutation has already happened and
no further modifications to the change are done during the reindexing.
Bug: Issue 15798
Release-Notes: cache the resolution of change /meta ref upon interactive indexing
Change-Id: If137693293e3a6a5aa51babb5576690878d827d3
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
This reverts commit 23db8a546b800b2c91629d80b6f5117ff635238a.
Reason: refs caching on stable-3.2 can be achieve using [1].
[1] https://gerrit.googlesource.com/modules/cached-refdb/
Release-Notes: skip
Change-Id: I44ad6fbe165bed57c3d639a6a0878f704139b7b3
|
|\| | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.1:
Cache change /meta ref SHA1 for each REST API request
Release-Notes: skip
Change-Id: I858522129b67f1e14720728f5d00b0f7c75f933e
|
| |\| |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.0:
Cache change /meta ref SHA1 for each REST API request
Release-Notes: skip
Change-Id: Ifaca2a4a00f9b7a3c9bb72f9a2b0d95c8c039009
|
| | |\|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-2.16:
Cache change /meta ref SHA1 for each REST API request
Release-Notes: skip
Change-Id: I00882cba94ae51998ad0b34ab17b0fa4e2f4ecf5
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Some Gerrit REST APIs may cause multiple lookups of change /meta
ref name to SHA1 during the same request, which could be as slow
as 400 ms per lookup for a 500k refs repository on a shared
NFS volume.
Make sure that a lookup of a /meta ref happens only once per GET/HEAD
API request by caching the result in a PerThreadCache scoped to the
execution of the RestApiServlet.
The measured improvement for a change reindex API (repo with 500k
refs stored on an external NFS volume with trustfolderstat=false)
is from 5s down to 500ms (10 times faster).
For other APIs or local storage with trustfolderstat=true,
the benefits are much more limited.
NOTE: The improvement on all other APIs (PUT, POST, DELETE, PATCH)
is outside the scope of this change, because it would imply a more
complex eviction mechanism due to the mutation of the /meta ref
during the potential NoteDb manipulation performed.
Bug: Issue 15798
Release-Notes: cache the resolution of change /meta ref on REST API
Change-Id: If41377b78cb0bbbde0fb22489a47f7634248e6f6
|
|\| | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
* stable-3.1:
Revert "CmdLineParser: Remove unused prefix argument"
Log the exception that caused the user's deactivation to fail
Avoid re-reading refs in schema 161
Run background GC in Schema 144 and update GCs done in Schema 146
Trigger All-Users GC in background in schema 123
Release-Notes: skip
Change-Id: I616eb660df12ab0bdcaa22c1a290a023a95cc0a1
|