| Age | Commit message (Collapse) | Author |
|---|
|
When there are many subdirectories in a folder, GetTreeEntries can get
bogged down trying to flatten all of them. Limit the number of flattened
calls to 100 to avoid this.
Relates to https://gitlab.com/gitlab-org/gitaly/-/issues/3445
Changelog: fixed
|
|
gitpipe: Prioritize context cancellation
Closes #3693 and #3697
See merge request gitlab-org/gitaly!3658
|
|
Bump actionpack, actionview, activesupport to 6.1
See merge request gitlab-org/gitaly!3661
|
|
featureflag: Default-enable LFS pointers pipeline
See merge request gitlab-org/gitaly!3653
|
|
This is necessary for us to support Ruby 3.
Changelog: changed
|
|
We're observing flaky tests in our gitpipe code, where the race happens
on context cancellation: we'll either see that the pipeline has shut
down gracefully, or alternatively we'll see that git-rev-list(1) was
killed because of context cancellation. This race can happen because we
don't prioritize handling of context cancellations: if git-rev-list(1)
was killed via the context, then it must've happened via our context and
thus we know that at the point of sending the error down the pipeline,
that the context has terminated already.
Fix the race by prioritizing context cancellation: before trying to send
down any results or errors, we'll first check whether the context was
cancelled. This also allows us to get rid of the workaround we had in
our pipeline tests where there was a special child context such that we
didn't observe killed git-cat-file(1) processes.
|
|
We have multiple ad-hoc implementations of senders for
CatfileInfoResuls. Deduplicate them into a single function.
|
|
Update google-protobuf to v3.17.1 and labkit-ruby to v0.20.0
Closes #3695
See merge request gitlab-org/gitaly!3656
|
|
Delete records of all replicas that were deleted transactionally
See merge request gitlab-org/gitaly!3654
|
|
`Gemfile.lock` was not updated properly, and we should update
google-protobuf to the version used by GitLab Rails.
Closes https://gitlab.com/gitlab-org/gitaly/-/issues/3695
Changelog: changed
|
|
Some test tidy for grpc-proxy on ProxyHappySuite
See merge request gitlab-org/gitaly!3606
|
|
[ci skip]
|
|
[ci skip]
|
|
ListConflictFiles: Allow tree conflicts when specified
See merge request gitlab-org/gitaly!3648
|
|
We recently added transaction support for repository deletions to alleviate
races that occur when quickly deleting a repository and creating it again.
As the deletes were not transactional, a deletion job to a secondary would be
scheduled and could be applied to the newly created repository if it is created
again on the secondary that is pending the deletion job. While most of the other
parts of making deletes transactional are in place, we don't currently delete
the database records of all of the replicas that were deleted transactionally,
only the primary's. This then leaves some stale state in the database that can
affect the newly created repository. This commit fixes this by deleting the
database records of all replicas that participated in the delete transaction.
|
|
DeleteRepository deletes a repository's database records. It deletes
the repository's entry in the `repositories` table which marks the
repository as having been deleted. It also deletes the primary's record
in the `storage_repositories` table to indicate the primary's replica
has been deleted. Right now it returns a repository doesn't exist error
if neither of those database records exist. The error isn't really accurate
as it always includes the storage as well which isn't relevant if the
repository doesn't exist at all. With transactional deletes, this is going
to be even more inaccurate as we are deleting records for multiple storages
in one go. To make this a bit nicer, this commit changes the returned error
to be a more generic no rows affected error.
|
|
coordinator: Fix repo creation/removal race for up-to-date secondaries
See merge request gitlab-org/gitaly!3603
|
|
When creating repositories, Rails will in some cases first schedule a
call to `RemoveRepository()` to make sure that there's no old repository
obstructing the path we are about to create the repository at. Given
that `RemoveRepository()` isn't transactional, this action will be
replicated to secondaries asynchronously. With our current design, these
replication jobs will not cause a bump of the repository generation
given that we may not even have an entry for the repository. Combined
with the fact that we do not take outstanding replication jobs into
account when computing whether a node is up-to-date or not, this means
that secondaries will be considered up-to-date even though they have
deletions pending. This results in a race between the replication jobs
which are about to delete the target repository and the subsequent call
to `CreateRepository()` et al. If this race is lost (that is, the
deletion wasn't scheduled before `CreateRepository()` gets executed,
which is almost always), then we will end up with the repository getting
deleted during or after its creation.
This race needs to be fixed via two changes:
1. We need to take outstanding modifying replication jobs into
account when computing whether a node is up to date or not.
Otherwise, we may be in the middle of processing a transactional
RPC when a replication job kicks in and modifies repository state
while we're operating on it.
2. With (1) implemented, we need to make `RemoveRepository()`
transactional, otherwise it would always cause secondaries to be
considered out-of-date and repository creation wouldn't use
transactions in most cases.
This commit implements (2) and enables transactional behaviour for
`RemoveRepository()` which has been implemented in the preceding commit.
Note that this requires a modification of `TestRemoveRepository()`:
previously, we didn't set up the backchannel handshaker and thus
couldn't accept votes from Gitaly nodes. Now that we have transactions
enabled for this RPC, we must be able to handle votes or otherwise the
RPC is slated for failure. We thus manually need to set up the node
manager to have a backchannel handshaker.
Changelog: fixed
|
|
The `RemoveRepository()` RPC currently does not support transactional
voting. This is now implemented by doing a vote both previous to
removing the repository and after the repository either has been removed
or found to not exist.
Transactional behaviour is not yet enabled in Praefect and thus this new
logic isn't used. This will be handled in a subsequent commit.
Changelog: added
|
|
This commit contains some preliminary refactorings to make the
`RemoveRepository()` RPC follow best practices and make it a bit more
readable. No functional changes are expected from this commit.
|
|
The map which specifies mutating RPCs and whether they should use
transactions or not currently claims that the reason for disabled
transactional behaviour is that the RPCs at hand simply don't modify
references and thus shouldn't use transactions. This line of reasoning
doesn't hold anymore: we have several RPCs which don't modify refs, but
instead perform manual voting on different state.
Split up the list of disabled RPCs into two:
- The first list contains all the RPCs where we didn't implement
manual voting logic yet, but which may be changed in the future to
support transactional behaviour.
- The second list contains RPCs which are considered idempotent
"optimizing" RPCs: while they do modify on-disk state, the result
as visible to the user should always remain the same. Those RPCs
are special cased to not cause a repository generation bump and
thus retaining non-transactional behaviour is fine there.
This should hopefully represent the current state much more accurately.
|
|
Some of our Praefect server tests use `testserver.RunGitalyServer()` to
setup their Gitaly nodes. While this isn't a problem by itself, this
function will by default set up a Praefect proxy for the Gitaly nodes if
running the "test-with-praefect" target. The result is that Gitaly nodes
are proxied by two Praefects: first the one set up by above function,
and then the one we're setting up ourselves in the tests. This is not a
supported configuration and may cause RPCs to fail in case Gitaly needs
to reach out to Praefect, e.g. for transactional voting.
Fix the issue by passing the `WithDisablePraefect()` option.
|
|
When using the Praefect proxy for our tests, then Praefect will use
Gitaly's health information to inform routing decisions. As a result, we
must make sure that Gitaly is in fact healthy before the tests start,
otherwise routing may fail.
Call `waitHealthy()` for the Gitaly connection to ensure that this is
the case. In order to avoid code duplication, `waitHealthy()` is
refactored to perform the dialling itself.
This change requires us to get rid of the manually configured listening
address in `TestCoordinator_grpcErrorHandling()` as it keeps us from
connecting to the Gitaly node's health service. It doesn't seem to serve
any immediate purpose anyway, so it's not much of an issue in the first
place.
|
|
The feature flag for use of the LFS pointers pipeline has rolled out to
production since June 29th without any issues. Given that this guards
code of an accessor and not a mutator, switch it to default-enabled.
Changelog: changed
|
|
|
|
[ci skip]
|
|
[ci skip]
|
|
[ci skip]
|
|
Refine metrics descriptions
See merge request gitlab-org/gitaly!3652
|
|
Fix issues with replication of optimizing RPCs
See merge request gitlab-org/gitaly!3638
|
|
Changelog: other
|
|
In https://gitlab.com/gitlab-org/gitlab/-/issues/281171, we want
to display what type of conflict occurred for a specific file.
There are cases that there will be a missing side (e.g. tree-based
conflicts like renames) and currently ListConflictFiles RPC only
allows returning of conflicts when there is no missing side.
This MR changes ListConflictFiles to accept an AllowTreeConflicts
param which will then be set to true on gitlab-rails when we want
to get the list of conflicts.
Changelog: changed
|
|
Implement optimized RSS monitor based on /proc/[pid]/mstat
See merge request gitlab-org/gitaly!3646
|
|
gitpipe: fix type name conflict after merge
See merge request gitlab-org/gitaly!3651
|
|
Backup in parallel by storage
See merge request gitlab-org/gitaly!3526
|
|
In 8c79cea06c7546ce97d6d27d33e79a5b626a41e9, we renamed Revlist* types
to Revision* as the types are not only used with git rev-list. During the
review, some more test cases got added which use the old type name. This
then caused build to fail. This commit fixes the semantic conflict and
uses the new type name.
|
|
ref: Reimplement ListAllTags via object pipeline
See merge request gitlab-org/gitaly!3645
|
|
commit: Allow listing commits in reverse
See merge request gitlab-org/gitaly!3650
|
|
[ci skip]
|
|
git: Accept commits and tags with malformed signatures
Closes #3687
See merge request gitlab-org/gitaly!3640
|
|
SetConfig: implemented config setting using git2go (implemented after config voting)
Closes #3029
See merge request gitlab-org/gitaly!3536
|
|
The goal of this change is to relieve pressure on forkExec mutex on platforms where this is supported.
Changelog: performance
|
|
Conflicts:
internal/metadata/featureflag/feature_flags.go
Conflicts resolved by ZJ
|
|
praefect: Fix incorrect error tracking for secondaries
See merge request gitlab-org/gitaly!3620
|
|
We're currently having a lot of scalability issues with ListAllTags in
production. One of the root causes of this is likely going to be the IO
patterns we use: we start git-for-each-ref(1), and for each tag returned
we'll request the object info from git-cat-file(1) and then, depending
on the object type, we'll ask another git-cat-file(1) process for its
data. In case it's an annotated tag, we manually peel the tag until we
hit a non-tag object. We're thus bouncing between these three processes
all the time in a sequential way, which is extremely inperformant.
Now that we have extended the gitpipe package to support enumerating
references via the new `ForEachRef()` step, let's convert the code to
use a pipeline which drives all three processes in parallel. This should
prove to be much more efficient given that it fixes above IO patterns.
As an additional low hanging fruit, the new code transforms output from
git-for-each-ref(1) to request both the tag as well as its peeled
non-tag object in case it's an annotated one. While this does additional
work in the context of lightweight tags given that we request the same
object twice, this shouldn't be much of a problem given that the
previously parsed object is still going to be in git-cat-file(1)'s
object cache. On the other hand, this is going to be a lot more
efficient in case we do have an annotated tag given that we don't have
to manually peel the tag to its target anymore, but simply have git do
it for us. This also lifts the current limit of nested tags.
All in all, this should result in a nice performance boost for the RPC.
Given that this may result in incompatibilities, the new implementation
is hidden behind a feature flag for now.
Changelog: performance
|
|
Implement a new revisions pipeline step which lists references via
git-for-each-ref(1): given a set of patterns, it will iterate over all
references which match this pattern.
|
|
While we already have a filter function for the revision pipeline which
allows the caller to purge revision results, a future user will require
the ability to insert additional revisions into the pipeline.
Add a new pipeline step `RevisionTransform()`: given a single item, it
returns a slice of items which replace the current one. This can either
be the item itself in case of an identity mapping, an empty slice to
drop the item or multiple items to add additional revisions into the
pipeline.
Given that this can easily be used to implement `RevisionFilter()`,
this funciton also refactors that function to be implemented in terms of
`RevisionTransform()`.
|
|
The code to send RevisionResults down the pipeline is kind of convoluted
given that we need to watch out for context cancellation. Pull it out
into a standalone function such that it can easily be reused by future
callers.
|
|
While some part of the Revlist pipeline step is indeed specific to
git-rev-list(1), much of it is simply generic infrastructure to list
revisions. Given that we're about to add a second user of the generic
infrastructure which uses git-for-each-ref(1), let's rename the generic
parts to be called `Revision*` instead.
|
|
While the catfile package already implements the logic to parse tags,
this functionality is not exposed to users outside of the package. We'll
need this though in order to implement parsing of commits as part of the
gitpipe code, where we get an `io.Reader` as well as object information.
Refactor the code to pull out a new `ParseTag()` function which parses
raw tags from an `io.Reader`.
|
