Age | Commit message (Collapse) | Author |
|
If a rate limiter idicates no more traffic is allowed, Gitaly should
return a non-retryable error. gRPC Unavailable indicates a retry is
possible, so gRPC ResourceExhausted is more appropriate to return to the
client.
Changelog: changed
|
|
When the concurrency queue has reached its limit, Gitaly should return a
non-retryable error to the client. gRPC's ResourceExhausted indicates
the system is under-resourced and that the request should not be retried
at will.
Changelog: changed
|
|
|
|
|
|
Handle DeleteObjectPool calls in Praefect
Closes #3742 and #4078
See merge request gitlab-org/gitaly!4395
|
|
Remove implicit pool creation on link behavior
See merge request gitlab-org/gitaly!4455
|
|
featureflag: Remove TransactionalSymbolicRefUpdates featureflag
See merge request gitlab-org/gitaly!4467
|
|
Add RateLimiting
Closes #4026
See merge request gitlab-org/gitaly!4427
|
|
Add a rate limiting middleware into the interceptor chain for a Gitaly
server.
Changelog: added
|
|
Now that we are adding a second limit handle, adjust the code to allow
for multiple limit handlers to be passed into a server invocation.
|
|
RateLimiter contains a limiter per rpc/repo pair. We don't want this to
grow monotinically since it will incur a heavy memory burden on the
machine. Instead, introduce a background process that looks through the
limiters and removes the ones that have not been used in the past 10
refill intervals.
|
|
Introduce a simple rate limiter that limits the number of requests a
minute that an RPC can allow.
If the feature flag is enabled, the middleware will drop any request
that bursts the per second limit of the RPC. Otherwise, it will only emit
metrics so we can first have some data on the traffic profile.
|
|
[ci skip]
|
|
This feature flag has been set to default on, and deleted from
production. There have been no observable issues, so it's now safe to
fully remove the feature flag.
Changelog: changed
|
|
commit: Add CheckObjectsExist RPC
Closes #3986
See merge request gitlab-org/gitaly!4450
|
|
When pushing commits to a repository, access checks are run. In order to
use the quarantine directory, we need a way to filter out revisions that
a repository already has in the case that a packfile sends over objects
that already exists on the server. In this case, we don't need to check
the access.
Add an RPC that when given a list of revisions, returns the ones that
already exist in the repository, and the ones that do not exist in the
repository.
Changelog: added
|
|
Allow Commit.RawBlame to take a Range parameter
See merge request gitlab-org/gitaly!4433
|
|
Before, concurrency limiting was the only limiting middleware. The name
of the metric had gitaly_rate_limiting in it, which was a bit of a
misnomer since rate was never part of the equation. Now that we are
actually adding a rate limiter to Gitaly, the concurrency metrics will
be mistaken for rate limiting metrics.
Change the name of these by replacing rate_limiting with
concurrency_limiting.
Changelog: changed
|
|
A future commit will add a new middleware that will limit based on the
rate rather than concurrent calls. There is a good amount of logic
currently used by the concurrency limiter that can be reused since a
rate limiter is also operating on incoming requests based on RPC name.
To make easier to add this new limiter type in the future, refactor the
code by adding some abstractions easier to add another type of limiter.
|
|
To prepare for a rate limiting middleware, add a struct to support
configuring a rate limiter. Behind the scenes, we are using
golang.org/x/time/rate package, which implements rate limiting with a
concept of a token bucket. There are two relevant values. Burst refers
to the maximum size of the token bucket. For a request to be handled, a
token is retrieved from the token bucket. Once the bucket is empty, no
more requests can be handled.
The token bucket will be refilled to capacity as defined by "Burst"
according to what is set by "Interval".
Changelog: added
|
|
Git supports a wide array of range options for this option, but for now
we restrict the formats we support to the very simplest, which is of
the form "1,100".
We can use this to implement limits on how many lines of the blame we
retrieve GitLab-side, which should help to reduce the resource usage
of this particular RPC.
Changelog: added
|
|
|
|
Use rev-list --all --objects --disk-usage to calculate repository usage
See merge request gitlab-org/gitaly!4430
|
|
In the previous commit, we added a Size() method in localrepo that calls
git rev-list --all --objects --disk-usage to calculate the size of the
repository.
Add a feature flag that, when toggled on, will cause RepositorySize to
use this new way of calculating repository size.
Changelog: added
|
|
Add a Size() method for repo that calculates disk usage of objects. The
way we currently calculate repository size is to use du(1), which is a
quick and dirty way to get the total storage of a repository. However,
this ends up varying depending on the __how__ the objects are stored by
Git, which is affected by repository maintenance strategies and can vary.
A more consistent way to calculate storage is to get a total of number
of reachable objects with git rev-list --all --objects --disk-usage
|
|
ssh: Clean up output when pre-receive hook fails
See merge request gitlab-org/gitaly!4318
|
|
When the pre-receive hook failed, git-receive-pack(1) exits with code 0.
It's arguable whether this is the expected behavior, but anyhow it means
cmd.Wait() did not error out. On the other hand, the gitaly-hooks
command did stop the transaction upon failure. So this final vote fails.
To avoid this error being presented to the end user, ignore it when the
transaction was stopped.
Issue: https://gitlab.com/gitlab-org/gitaly/-/issues/3636
Changelog: fixed
|
|
Add test to check if the error message from a failing custom pre-receive
hook is presented to the user.
|
|
In a future commit we'd like to process the stdout and stderr output
from the ssh push command separately. With this commit the setting up of
the command is extracted into a function so in the future it's possible
to specify where to write stdout/stderr to.
|
|
Praefect is currently disabled in RemoveRepository tests which allows
for Praefect's RemoveRepository behavior to deviate from the Gitaly's.
This commit enables Praefect for these tests so we can be sure we catch
behavior deviations between the two handlers.
|
|
Praefect contains two special handlers for handling repository
removals. Both of these handlers do largely the same thing. This
commit shares the common code between the two handlers.
|
|
Praefect currently proxies DeleteObjectPool calls to Gitalys like
any other mutating RPC. This has the same problem as RemoveRepository
previously had; a pool can be deleted from the disk and it's metadata
record still left in the database. This can then cause problems as
Praefect believes a pool replica still exists where one doesn't exist.
Praefect doesn't even treat DeleteObjectPool as a repository removing
RPC. This means the deletions have never been replicated to the
secondaries and the pool metadata records have been left in place. This
then causes the reconciler to repeatedly attempt to reconcile from a
non-existing primary pool replica to the secondaries.
This commit fixes both issues by handling pool deletions in Praefect.
Similarly to RemoveRepository, the metadata record of the pool is first
deleted and only then the pool is attempted to remove from the Gitalys
that have it. This ensures atomicity of the removals when Praefect is
rewriting the replica paths.
With the behavior fixed, the Praefect specific test asserations can be
removed as the behavior now matches what how a plain Gitaly would
behave.
The handler in Praefect is tested via the same tests that test the
handler in Gitaly. Adding separate tests doesn't make sense as external
behavior of the the handlers should match and the handler shouldn't
exists in Praefect if it is removed from Gitaly.
Changelog: fixed
|
|
DeleteObjectPool's tests do not currently test what happens if the
request is missing the pool repository. This commit adds the missing
test case. While at it, it exports the functionality to extract a pool
from the request so Praefect's DeleteObjectPool handler can reuse it
later and pass the same test.
|
|
TestDelete does not verify whether the object pool exists or not
after the call to DeleteObjectPool. This is somewhat of an imporant
assertion to make to ensure the deletion logic is working correctly.
This commit adds that missing assertion.
|
|
This commits exports the IsRailsPoolPath function from the housekeeping
package so the upcoming DeleteObjectPool handler in Praefect can
use it to determine whether the object pool has a valid path.
|
|
Add logging to gitaly-git2go
Closes #3229
See merge request gitlab-org/gitaly!4426
|
|
Makefile: Fix performance issues caused by tracing in binaries
Closes #4020
See merge request gitlab-org/gitaly!4451
|
|
We have observed multiple times already that short-lived Go binaries
spawned by Gitaly have major initialization overhead because they need
to initialize the `awk-sdk-go` dependency. This dependency has megabytes
of maps containing all the different datacenters of AWS, and this cost
quickly adds up in case the binary only does very limited work. As a
result, we have seen runtime of `gitaly-hooks` and `gitaly-lfs-smudge`
to be dominated by this initialization.
The thing though is that neither of these has a direct dependency on
`awk-sdk-go`. Instead, it is being pulled in as an indirect dependency
via `labkit`, but only if compiled with tracing enabled. And we do in
fact compile all binaries with tracing enabled by default.
Ultimately, having tracing in such short-lived binaries isn't all that
helpful, and in combination with the performance issues we're observing
it is time to drop the tracing infrastructure there. We cannot drop the
dependency on `labkit` though: it's required so that we can inject and
extract the correlation ID via the environment. But luckily, tracing is
only enabled when specific build tags are set. Conversely, if they're
unset, we also don't pull in the problematic `awk-sdk-go` package.
Fix the performance issue by splitting up Go build tags to be per
binary. Like this, we can keep tracing enabled in both Praefect and
Gitaly while we disable it for all the other binaries. Furthermore, this
also allows us to inject the Git2go-specific build tags only for the
`gitaly-git2go-v14` binary.
Changelog: fixed
|
|
We have recently introduced support for generating build IDs in Gitaly.
The biggest downside of this support is that we're forced to build all
binaries with separate commands, which is quite a lot slower by default
than when we built all commands with a single execution of the Go
compiler. Because of this we have added a fallback case that allows
developers to skip generation of the build ID so that local development
isn't significiantly slowed down.
Unfortunately, we now have another nail in the coffin for building all
binaries with a single command: we need to be able to specify separate
Go build tags per Go executable. This becomes unworkable though in the
case where we build them all in one go.
Luckily though, Gitaly's Makefile has recently also dropped the
`.NOPARALLEL` special target. With this target gone, we can parallelize
compilation of the different Go binaries via make(1) itself. This brings
us back most of the performance we would loose when we build binaries
with separate invocations of the compiler.
So let's drop the workaround which allowed developers to skip generating
the build ID. Instead, they should simply ask make(1) to parallelize
compilation via the `-j` flag. This change both simplifies our Makefile
and at the same time paves the way for per-binary Go tags.
|
|
Building Gitaly's Go executables is extremely verbose when generating
build IDs. Silence the output so that important signals aren't drowned
out by the noise.
|
|
While Gitaly's Makefile traditionally had the `.NOPARALLEL` special
target which restricted us to never parallelize jobs, this is not really
needed nowadays anymore. Targets should have proper dependencies and as
such it should be possible to use multiple jobs without breakage.
Let's convert our CI jobs to use multiple make jobs to speed it up.
|
|
Right now the `build` target depends on libgit2, which ultimately means
that we'll only start building Gitaly's binaries when libgit2 has been
compiled already. This is needlessly restrictive though: we have only a
single binary which depends on libgit2, which is `gitaly-git2go`.
Explicitly mark these dependencies such that we can parallelize building
libgit2 and some of our Go binaries.
|
|
Makefile: Fix indeterministic sorting order of Git patches
Closes omnibus-gitlab#6761
See merge request gitlab-org/gitaly!4458
|
|
With 3f6d4ca0f (Makefile: Group Git patches by version, 2022-03-31) we
have started to collect Git patches we want to apply by using the
`wildcard` function. This has broken compilation downstream though in
both CNG and Omnibus, where patches seemingly don't apply correctly
anymore.
The root cause of this may be an incompatibility in GNU Make: I'm using
GNU Make 4.3 on my system, which is sorting files returned by `wildcard`
[1]. Reversely, any systems which use older versions do _not_ sort them,
and thus the order in which we apply patches is indeterministic.
This has two consequences:
1. Applying patches may fail altogether because they are dependent
on one another.
2. With intedeterministic sorting order we may repeatedly rebuild
the Git versions because our `.version` file changes every time.
Fix this incompatibility by sorting the patches.
[1]: http://git.savannah.gnu.org/cgit/make.git/tree/NEWS#n187
Changelog: fixed
|
|
[ci skip]
|
|
git: Make Git v2.35.1.gl1 the default Git distribution
Closes #4087
See merge request gitlab-org/gitaly!4454
|
|
|
|
smarthttp: Fix test race where response buffer may not be filled
See merge request gitlab-org/gitaly!4447
|
|
If a repository is being linked to an object pool that does not exist,
the current behavior is to create the pool implicitly. This doesn't
bode well with Praefect as it doesn't know whether a pool was or wasn't
created and can't thus create a metadata record for the pool. The
functionality was removed behind a feature flag. It doesn't seem like
anything relies on the implicit pool creation, so this commit removes
the feature flag and returns an error when linking a repository to a
non-existent pool.
Changelog: changed
|
|
Now that we have rolled out bundled Git to production and that the
feature flag is gone we can treat this new version as stable. Let's thus
also upgrade our default Git distribution to v2.35.1.gl1 so that distros
which don't use bundled Git can also benefit.
Changelog: changed
|