| Age | Commit message (Collapse) | Author |
|---|
|
GetDefaultBranch prefers returing main over master. So setting the
default branch to master in tests leads to discrepancies.
|
|
workflow: Add "customer" label to Gitaly support template
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5465
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: John McDonnell <jmcdonnell@gitlab.com>
Co-authored-by: Andras Horvath <ahorvath@gitlab.com>
|
|
go: Update module github.com/pelletier/go-toml/v2 to v2.0.7
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5467
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
ruby: Update dependency google-protobuf to '~> 3.22.1'
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5471
Merged-by: Stan Hu <stanhu@gmail.com>
Approved-by: Stan Hu <stanhu@gmail.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
|
|
go: Update module github.com/jackc/pgx/v4 to v4.18.1
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5463
Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: James Fargher <proglottis@gmail.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
ruby: Update dependency gitlab-labkit to '~> 0.31', '>= 0.31.1'
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5466
Merged-by: Stan Hu <stanhu@gmail.com>
Approved-by: Stan Hu <stanhu@gmail.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
|
|
catfile: Don't return readers to cache when their queues are in use
Closes #4321
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5439
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
|
|
|
|
operations: Remove UserCommitFilesStructuredErrors ff
Closes #4472
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5450
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Will Chandler <wchandler@gitlab.com>
|
|
tracing: Fix disconnected spans in gitaly-hooks
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5419
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: John Cai <jcai@gitlab.com>
Co-authored-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
|
|
gitaly-hooks is a sanitized environment. It does not initialize tracing
nor export any tracing spans, even though its process environment
variables set includes tracing info. The hook process then triggers RPC
call back to Gitaly. Those RPCs are handled, but the corresponding spans
are disconnected from the original source. This behavior makes a full
flow involving gitaly-hooks broken and scattered all over the place.
This commit fixes this situation by implementing two passthrough gRPC
interceptors. They read the tracing info from env and then write into
outgoing metadata without modification.
|
|
|
|
testdb: Remove global advisory lock when creating Praefect database
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5438
Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
'4738-support-setting-default-branch-through-the-write-ahead-log' into 'master'
gitaly: Add functionality to update default branch
Closes #4738
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5412
Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Reviewed-by: James Fargher <proglottis@gmail.com>
Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Karthik Nayak <knayak@gitlab.com>
|
|
Revert "Merge branch 'jc-submodule-in-git-fix' into 'master'"
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5461
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>
|
|
This reverts merge request !5448
|
|
repository: Handle empty root trees in GetArchive
Closes #4786
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5455
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Will Chandler <wchandler@gitlab.com>
|
|
housekeeping: Let strategies control expiration grace period
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5453
Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
gitaly/server: Fix leaking Goroutine in server factory test
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5452
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
|
|
operations: Reimplement UserUpdateSubmodule without git2go with fixed error message
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5448
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>
|
|
go: Update module github.com/stretchr/testify to v1.8.2
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5459
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
tools/golangci-lint: Update module golang.org/x/tools to v0.6.0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5430
Merged-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
featureflag: Default enable `replicate_repository_hooks`
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5449
Merged-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
Refactor the `PruneUnreachableObjects()` RPC to use the newly introduced
`housekeeping.PruneObjects()` function.
|
|
When pruning objects we have a default grace period of two weeks. This
grace period is dictated by `housekeeping.OptimizeRepository()`, which
has this limit hardcoded. This has two issues:
- `OptimizeRepository()` should not be responsible for the policy of
when exactly we perform certain optimizations. This is the job of
the optimization strategy instead.
- We pass the expiration grace period as approxidate to git-prune(1)
and thus have no fine-grained control of which objects exactly get
pruned. This will become a problem once we introduce cruft packs,
where we will then need to use the same date both when creating
the cruft packs and when pruning objects.
Introduce a new `PruneObjects()` helper function along with a config
struct that allows the caller to set the exact date before which objects
shall be pruned. This allows us to easily move control of that expiry
date into optimization strategies.
|
|
|
|
Updating a submodule is simply updating the oid of the commit at the
path. We can do this by manually rewriting trees and writing a new
commit. If we impelement it this way, we don't need to rely on Git2Go.
|
|
When we call git-ls-stree(1), we use the form <treeish>:<path>. When "."
is used for <path>, git rejects it as an invalid object. Fix this by,
automatically rewriting "." to "" since callers might pass in ".".
|
|
Add a feature flag that will be used to gate the implementation of
submodule to not use git2go, but instead native git plumbing commands.
|
|
To prepare for a pure git implementation of update submodules without a
worktree, first refactor the current git2go implementation into its own
function.
|
|
Currently requesting GetArchive of a full repo on a commit with an empty
root tree (i.e. an empty project) `git archive` will error out with
fatal: pathspec '.' did not match any files`. This will result in an
`Internal` error being returned, leading to spurious incident reports
for the SRE team.
To resolve this, when the requested path is ".", the default for web and
API, validate that the root tree of the commit is not empty.
Changelog: fixed
|
|
Extract repository locking into a helper
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5437
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>
|
|
lstree: Use localrepo.TreeEntry instead of lstree.Entry
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5416
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
|
|
The tests for the Gitaly server factory that verify the order in which
specific listeners are closed are using various different blocking RPC
calls to see which one completes first. These blocking RPC calls are
spawned in separate Goroutines, but the test does not synchronize with
all of these Goroutines to actually complete before the test case exits.
This causes a flake where one of the Goroutines sometimes observes the
context cancellation instead of the intended error:
panic: Log in goroutine after TestGitalyServerFactory_closeOrder has completed:
Error Trace: /builds/gitlab-org/gitaly/internal/gitaly/server/grpc.go:41
/builds/gitlab-org/gitaly/internal/gitaly/server/grpc.go:69
/builds/gitlab-org/gitaly/internal/gitaly/server/server_factory_test.go:222
/builds/gitlab-org/gitaly/internal/gitaly/server/asm_amd64.s:1594
Error: Should be empty, but was (*status.Status)(Inverse(protocmp.Transform, protocmp.Message{
"@type": s"google.rpc.Status",
- "code": int32(13),
+ "code": int32(1),
- "message": string("blocking RPC"),
+ "message": string("context canceled"),
}))
Fix this bug by properly waiting for all Goroutines to exit before
returning from the test.
|
|
The common test logic for listener in `verifyListener()` is spawning a
Goroutine in which the listener is set up. This Goroutine eventually
gets cancelled via context cancellation so that it is unblockend and
exits. And in order to synchronize with it exiting, we wait for a
channel to be closed by that Goroutine.
While this works as expected in the general case, there is one early
exit in the helper in case the caller does not pass a verifier as input.
As a result it can happen that we exit the test while the Goroutine is
still running, and that can then race with the database being killed.
This causes the following flake:
=== FAIL: internal/praefect/datastore TestListener_Listen_storage_repositories_verification_updates_ignored (2.77s)
listener_test.go:465:
Error Trace: /builds/gitlab-org/gitaly/internal/praefect/datastore/listener_test.go:465
/builds/gitlab-org/gitaly/internal/praefect/datastore/asm_amd64.s:1594
Error: Should be true
Test: TestListener_Listen_storage_repositories_verification_updates_ignored
Messages: wait for notification: FATAL: terminating connection due to administrator command (SQLSTATE 57P01)
Fix this flake by deferring the context cancellation together with
waiting for the channel to be closed.
|
|
The SQL statement to terminate database connections embeds the database
name that is to be terminated directly into the string. Even though we
know that the database name will always be a UUID that can be safely
passed, it still doesn't conform to best practices.
Refactor the code to instead use a proper parameter for the database
name.
|
|
|
|
Setting up the Praefect database for our tests works by populating a
shared template database with our migrations. This template database is
then used as the source of the actual database we're using for the test
and is shared across our tests. It is thus essentially a global shared
resource not only across tests of a single testrun, but even across
multiple runs of the testsuite. This brings multiple problems with it:
- We need to acquire a global advisory lock so that no two tests try
to update the template database at the same point in time. This
sequentializes tests which want to create a test database.
- There is a race between creating the template database and the
real database as we release the lock before we create the real
database. Consequentially, any concurrently running test with a
different set of migrations could modify the state of the template
database.
- We need to have a bunch of logic to detect when the template
database is not in a state that would be recognized by us.
Overall, this solution is kind of complex.
Now the question is why we have decided to do it like this in the first
place. The logic has been introduced via c61cdadb5 (Replace in-memory
queue with Postgres implementation, 2021-07-14), which does not mention
why we use a shared template database instead of just creating the final
target database directly. There are two reasons I can think of though:
1. It stresses the migration logic a bit more. This is in my opinion
not a good argument though as the migration logic should ideally
be tested as a standalone unit anyway.
2. We want to avoid having to re-run database migrations on every
test in order to be more efficient.
I doubt that (1) had been the reason, and assume it was (2). Benchmarks
show though that this optimization does not quite work in our favor:
Benchmark 1: with advisory lock
Time (mean ± σ): 12.042 s ± 0.487 s [User: 42.657 s, System: 8.606 s]
Range (min … max): 11.551 s … 12.643 s 5 runs
Benchmark 2: without advisory lock
Time (mean ± σ): 10.928 s ± 0.080 s [User: 43.569 s, System: 9.402 s]
Range (min … max): 10.816 s … 11.007 s 5 runs
Summary
'without advisory lock' ran
1.10 ± 0.05 times faster than 'with advisory lock'
As you can see, the tests run about 10% _faster_ without the advisory
lock. This is because the tests can now be parallelized better as they
do not depend on a global lock anymore. That being said, both user and
system time have increased as we now spend more time initializing the
database. But that increase is seemingly getting absorbed by improved
parallelization.
Remove the global advisory lock. This simplifies the code, reduces the
amount of global state we have and speeds up our tests.
|
|
gitaly-backup: Add -remove-all-repositories flag to restore
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5410
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Evan Read <eread@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: James Fargher <proglottis@gmail.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Evan Read <eread@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>
|
|
tools: Fix incompatible dependencies for new Gitaly linter
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5440
Merged-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
|
|
The -remove-all flag is a comma-separated list of storage names that
will have all their repositories removed before the restore starts.
Changelog: added
|
|
The manager will be used to clear storages prior to starting a full
backup.
|
|
praefect/server: Fix Goroutine leak in readiness test
Closes #4551
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5443
Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
|
|
We've been running `UserCommitFiles` with structured errors for some
time without issue, so let's go ahead and remove the feature flag.
|
|
|
