Age | Commit message (Collapse) | Author |
|
With OptimizeRepository becoming our new RPC which performs all
repository maintenance in one place it's harder to tell what exactly is
going on without proper metrics. One bit of information that gets lost
is whether repacks are incremental or full repacks, which is quite an
important distinction due to the impact on latency.
Expose this information both via logs and via Prometheus to keep better
track of it. While at it, alos exxpose information about whether we are
writing bitmaps or not.
Changelog: added
|
|
Expose the number of stale files we have pruned via a Prometheus metric
so that it's easier to see what kinds of garbage we frequently need to
clean up.
Changelog: added
|
|
We expose a Prometheus counter that is counting how many empty
directories we have pruned. This is a broader concept in our
housekeeping code, where we also prune other kinds of stale files.
Generalize the counter into a counter vector such that we can reuse the
same counter for all the different types of data we prune. While this
breaks the metric in case it was used anywhere, there are no references
to this counter across the complete GitLab group. Furthermore, we
haven't ever guaranteed backwards compatibility for metrics anyway.
Changelog: changed
|
|
While we have a central manager component which is supposed to hold all
state related to housekeeping, we missed to migrate one of our metrics
into it.
Migrate it to get rid of one more global variable.
|
|
Because we may be deleting files which cause reference directories to
become empty, pruning of empty reference directories needs to happen
last in our housekeeping tasks. Because of this we also don't log info
about pruned references in the log message which reports all the other
removals.
Refactor the code to return the number of pruned empty reference dirs
from `removeRefEmptyDirs()` such that we can log them in the calling
function with all the other cleanups.
|
|
The test which verifies that we correctly prune specific files is using
a subfunction to execute subtests, which makes the test hard to extend.
Refactor it by inlining the subtests.
|
|
In 889450266 (ci: Run tests as unprivileged user, 2022-01-13) we have
converted tests to run as unprivileged user. Back then we forgot to also
adjust the Coverage job though, which is still running as a privileged
user.
Convert the job to also run tests unprivileged. This fixes an upcoming
test failure we're about to introduce where housekeeping tasks remove a
file that it shouldn't be able to because of a lack of permissions.
|
|
repository: Add updateHeadFromBundle in CreateRepositoryFromBundle
Closes #4086
See merge request gitlab-org/gitaly!4401
|
|
|
|
[ci skip]
|
|
Extend invalid metadata deletion logic to repos existin on target
Closes #4083
See merge request gitlab-org/gitaly!4396
|
|
[ci skip]
|
|
proto: Add structured error types for UserRebaseCofirmable
See merge request gitlab-org/gitaly!4382
|
|
This commit introduces these changes by creating a new
UserRebaseConfirmableError Protobuf message which contains all potential
structured errors we want to return from the UserSquash RPC.
Changelog: added
|
|
Disable implicit pool creation on link behind a feature flag
See merge request gitlab-org/gitaly!4397
|
|
[ci skip]
|
|
doc: Document supported Git execution envinronments
See merge request gitlab-org/gitaly!4392
|
|
operations: Implement structured errors for UserSquash
See merge request gitlab-org/gitaly!4374
|
|
Document the different ways to access Git installations supported by
Gitaly. Most importantly, this also documents the way our new bundled
Git binaries work and why they were introduced.
|
|
[ci skip]
|
|
Add squash parameter to git2go merge
See merge request gitlab-org/gitaly!4241
|
|
cgroups: Remove paths field
See merge request gitlab-org/gitaly!4398
|
|
repository: allow CreateRepository to take default_branch
See merge request gitlab-org/gitaly!4385
|
|
The paths field on the CgroupV1Manager was being accessed concurrently,
leading to a panic. However, this field is not actually used by
anything. Fix this issue by removing the field.
Changelog: fixed
|
|
This commit adds a feature flag to disable the implicit pool creation
when joining a repository to a non-existent object pool. It looks like
this behavior is not needed given Rails should create the object pool
prior to considering it ready for joining. Disabling this behavior makes
it easier to handle pools in Praefect as Praefect needs to create metadata
records for all of the repositories. This implicit behavior might create
a repository without Praefect's knowledge. Special handling would be
required for this particular RPC only, so let's see if we can remove the
behavior rather than implement it in Praefect.
|
|
This change adds support for squashing in gitaly-git2go merge.
This will allow developing new RPC for squash that would be
more optimized, and wouldn't have to use rebase before
performing merge.
We simply use the same strategy as for merge, the only difference
is that we set only one parent to the merge commit.
Changelog: added
|
|
Squash commits in GitLab use different committer and author.
Committer is the user that merges, and author is the user
that created the MR.
As we want to add squashing to this command, we have to make
sure that we support Committer* arguments as well.
Committer variables are optional, author data is
used when they are empty.
That way this change is backward compatible.
However, if at least one of Committer* variables are set,
all of them have to be.
|
|
In Git, "ours" and "theirs" are named from the perspective of the
maintainer accepting the changes. Ours is the main target ref that
we merge into, and theirs is the source ref.
Based on orders of parents in call to CreateCommitFromIds
(ours, theirs) that is how we use those as well, only this comment
was wrong.
This is probably related to
https://gitlab.com/gitlab-org/gitaly/-/merge_requests/2868
and it was fixed in code, but the mistaken comment was left.
This is important to iron out before we add squashing,
as the wrong parent for squashed commit could be catastrophic.
|
|
Praefect contains logic to delete metadata record of a non-existent
source repository during replication. Praefect checks for the
ErrInvalidSourceRepository error, and if it receives it, it deletes
the metadata record of the source repository. Right now, the error
is only returned from ReplicateRepository if the source repository
didn't exist when the target is creating the repository for the
first time. This means Praefect's clean up logic doesn't run if the
target already has the repository on the disk but the source repository
doesn't exist. This can lead to reconciliation loops where Praefect
keeps trying to replicate from the non-existent source repository
over and over again as all of the replications fail. This commit
changes ReplicateRepository to return the error when the source
doesn't exist but the target repository does.
This logic was initially implemented to break the reonciliation loops
that occurred due to Praefect creating invalid metadata records when
a repository creation failed. As DeleteObjectPool RPC was not handled
as a repository deleting RPC in Praefect, the metadata records were
left in place which is currently causing reconciliation loops. Changing
the error return allows for Praefect to clean up those invalid metadata
records. In the long run, Praefect's background metadata verifier would
capture such issues and this logic could be removed.
Changelog: fixed
|
|
In GitLab rails, there is code to ensure that the default branch on a
new repository is set. This is currently done with an extra WriteRef
call. This is done in a racy way, which causes problems with transaction
voting. To fix this, we can simplify the logic altogether and avoid
adding another roundtrip, add a parameter in CreateRepositoryRequest
to pass a default_branch.
Changelog: changed
|
|
proto: Deprecate PreReceiveError fields
See merge request gitlab-org/gitaly!4393
|
|
With e542a7d87 (operations: Implement rich errors for UserMergeBranch
access checks, 2021-08-30) we have introduced the ode to return proper
errors when access checks failed via gRPC's rich error model. The
feature flag has been remove with a23b34894 (operations: Enable use of
detailed errors for UserMergeBranch, 2022-02-16) , which means that we
don't ever set the `PreReceiveError` field of the
UserMergeBranchResponse message anymore.
Deprecate the field such that we can eventually remove it.
Changelog: deprecated
|
|
The PreReceiveError field in the UserMergeToRefResponse isn't ever set
because the RPC call doesn't in fact perform any access checks at all.
Digging back through history this doesn't seem to be an oversight in the
Go port, but was the case in the Ruby implementation already. While the
initial implementation of UserMergeToRef in 8fbc337b8 (Allow merging to
a ref without changing the target branch, 2019-01-25) still used to call
hooks, this was explicitly changed with 19fdfa12a (Skip hooks for
UserMergeToRef RPC, 2019-06-14) to not do so anymore. The field thus
hasn't been set anymore for more than two years by now.
Deprecate the PreReceiveError field so that we can eventually remove it.
Changelog: deprecated
|
|
Rewrite relative path in RepositoryReplicas conditionally
See merge request gitlab-org/gitaly!4390
|
|
Rails is failing on RepositoryReplicas tests as it now checks the
replica path. However, the Rails tests do not configure a database
so Praefect can't get the replica path from there. It's not possible
to mock out the function with the interface provided either. To fix
the problem, this commit adds a config check which enables the
relative path rewriting only if repository-specific primaries are
enabled, which also indicates that a database must be configured.
|
|
Bump danger to 8.4.4
See merge request gitlab-org/gitaly!4388
|
|
|
|
Rewrite relative path in RepositoryReplicas
See merge request gitlab-org/gitaly!4384
|
|
git: Add support for bundled Git v2.35.1.gl0
Closes #4067 and #4068
See merge request gitlab-org/gitaly!4352
|
|
RepositoryReplicas RPC is currently not accounting for relative path
rewriting when sending the CalculateChecksum RPCs. This causes it to
fail when Praefect generates unique paths for the repositories. This
commit fixes the issue by sending the requests to the Gitalys with
the appropriate replica paths.
|
|
Praefect's test for RepositoryReplicas is not creating the repository
through the API. As such, it doesn't exercise the relative path
rewriting code and doesn't catch problems there. The QA tests caught
the fact that RepositoryReplicas is not doing relative path rewriting
yet. This commit updates the test first to create the repository through
the API and to account for the relative path rewriting when creating
a test commit in one of the replicas.
|
|
[ci skip]
|
|
Mark RefTransactionService as handled by Praefect
See merge request gitlab-org/gitaly!4383
|
|
This commit marks the RefTransactionService with the intercepted
option to denote it is handled by Praefect. Such services do not
need operation type annotations on the RPCs as they do not go through
the proxy. This should have no behavioral changes and mostly just
cleans up the code a bit.
|
|
We now have support in place for different bundled Git execution
environments, and a preceding commit has already started to build and
install bundled Git v2.35.1.gl1.
Add support for this new bundled Git execution environment behind a
feature flag.
Changelog: added
|
|
We're currently still stuck with Git v2.33.1.gl2 because we haven't yet
been able to roll out bundled Git to production, and we do want to make
use of feature flags to switch to newly introduced Git versions. Bundled
Git should soon be landing in production though, so this commit starts
to build and install Git v2.35.1.gl1 such that we're prepared to quickly
roll out bundled Git and then the new Git version.
Note that this also backports two patch series from the upcoming Git
v2.36.0 release:
- Patches 21-26 fix an issue with the files refs backend, which
would execute the reference-transaction hook for references that
were about to be deleted twice: once for the loose ref itself, and
once for the packed backend. This is a performance issue, but also
required us to have a workaround that filters out any reference
transactions which only consisted of deletes.
- Patches 27-34 fix the `--atomic` flag of git-fetch(1) to also
cover deletion of references and backfilling of tags. This has
caused excessive voting via reference transactions in production
because the reference-transaction hook was executed twice for each
reference modified via those mechanisms. It is thus a correctness
fix because we don't commit changes to disk anymore if any of the
other ref updates fails, and a performance fix for mirror fetches
because we perform less voting.
|
|
We are about to introduce the ability to build multiple different Git
versions, and this will require us to also have multiple different
source directories. This commit prepares for this by converting our
current targets which build Git into pattern-based targets such that it
is easily possible to use multiple different directories.
Note that this commit also changes the default source directory of the
current default "_build/deps/git" one to "_build/deps/git-v2.33.1.gl2".
The build directory shouldn't be accessed by any callers, so this is not
expected to be much of a problem. What is accessed though is the default
installation directory for Git, which is "_build/deps/git/install". This
location is thus retained to not break compatibility.
The downside is that we'll now have to download multiple copies of the
Git repository. On the other hand, we now don't have to rebuild Git
whenever we change between different branches which use different Git
versions because the source directories would ideally not be modified
anymore.
|
|
We're about to remove references to `GIT_SOURCE_DIR` because there will
soon be multiple different source directories, where we have one per Git
version we're building. Prepare for this by converting recipes to derive
the location via automatic Makefile variables.
|
|
We have split up computation of the Git version and its patches into two
different sites, even though we only ever use patches in case the user
didn't pass a Git version or explicitly asked us for the "default" Git
version. This needlessly complicates the setup by requiring another
variable which tells us whether we have to apply patches or not.
Merge both sites into one to simplify the code.
|
|
When building Git we define both the GIT_PATCHES and GIT_EXTRA_VERSION
variables to influence the way Git is built. This is currently done via
`ifdef` conditionals, which are notably evaluated at the time of parsing
the Makefile. As a consequence, even if their values change at a later
point, we'll already have stripped or retained the code snippets they're
guarding because they won't be reevaluated. This is not an issue
currently, but it is going to be an issue when we ruse the target to
build multiple different Git versions.
Prepare for this by dynamically testing whether those variables are set
or not as part of the recipe itself.
|