| Age | Commit message (Collapse) | Author |
|---|
|
Enabling this feature ensures that the default branch returned by gitaly
is always what is specified in HEAD without using any heuristics.
|
|
This test case assumes that if there is only one branch, then it will be
treated as the default branch.
|
|
Dangerfile: Add group::gitaly::git and group::gitaly::cluster
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5639
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Andras Horvath <ahorvath@gitlab.com>
Reviewed-by: Andras Horvath <ahorvath@gitlab.com>
|
|
datastore: Refactor consistent storages cache
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5729
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Reviewed-by: Toon Claes <toon@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
localrepo: Refactor and improvements to tree functions
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5642
Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>
|
|
When the lookup of cached consistent storages fails we need to lock the
cache in order to update it. The lock itself is supposed to only span
over a single key that consists of the relative path of the repository
we're about to look up so that concurrent callers can fetch consistent
storages for multiple repositories at once.
The lock is still too coarse though: while we indeed only create the
lock for the relative path, we lock on the level of the virtual
storages. In other words, if the same relative path exists on two
different virtual storages then the lock would noww span across both
virtual storages. This is needlessly broad and not really what the
code intends to do.
Fix this by refactoring the syncer to be per-virtual-storage.
Changelog: performance
|
|
The cache for consistent storages will only fetch consistent storages
when it got an invalidation notification. It thus got two modes: either
it can look up values from the cache, or it needs to fetch them anew.
This logic is split up across three different functions with some code
flow that is not exactly easy to follow as locking code is transgressing
across multiple calling contexts.
Refactor the code to instead be contained in a single function, only.
This makes it easier to follow and makes it more obvious how locking is
supposed to work.
|
|
The cache for consistent storages uses a `cacheValue` type to represent
cached information. This name is too generic to be useful as it only
tells the reader that it has something to do with caching, but it does
not give any hint at what kind of information is actually getting
cached.
Rename the type to `cachedReplicaInfo` to further clarify its intent.
|
|
In v2 of the upstream `golang-lru.Cache` type, the structure was
refactored to make proper usage of generics. And while we have adapted
the code to that change, the result still relies on using `interface{}`
instead of using properly typed values. This decreases type safety and
increases the mental burden when reading the code.
Refactor the cache to instead use a properly data type.
|
|
The repository store interface uses plain maps as sets. Refactor them to
instead use our new set type. This reduces the mental overhead it takes
to first understand what the map is actually representing.
|
|
The idiomatic way to implement sets in Go is to use maps that have an
empty structure as value. While this works alright, it can be hard to
read and doesn't provide a lot of niceties that sets can typically
provide.
With the recent addition of generics though the way generic data
structures can be implemented has been significantly improved. This also
changes what is idiomatic in favor of better encapsulated generic data
types that provide a higher-level abstraction level.
Introduce a new generic set type as a first internal implementation of
such data structures.
|
|
Since WriteTree() operates on trees, change it to Write() and make it a
function for TreeEntry. It will do a depth first walk of all the Entries,
writing new trees as necessary. If a TreeEntry's OID is empty, then
Write() will attempt to write a new tree with all of its Entries.
|
|
Currently, a TreeEntry is a flat data structure in the sense that it
represents a single entry but does not have any children. A git tree
object has children though.
Change this by adding an Entries member to the TreeEntry struct. This
way, we can represent a real tree data structure.
Also, add a GetTree() function that can get an entire tree. We can
consolidate ListEntries() into GetTree().
|
|
'5027-flaky-test-testserver_postuploadpacksidechannel_gitconfigoptions-no_config_options' into 'master'
smarthttp: Skip flaky test in testServerPostUploadPackGitConfigOptions
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5740
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
|
|
operations: Drop unused param repoPath for merge
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5739
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Xing Xin <xingxin.xx@bytedance.com>
|
|
Cgroups: enable feature flag by default
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5738
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Steve Azzopardi <sazzopardi@gitlab.com>
|
|
These two new groups are going to replace gitaly::git and
gitaly::cluster. See https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_requests/122793
for details. Add them to the INHERITABLE_LABELS so they get
inherited from issues.
Also the group::gitaly label can also be inherited, in the situation
that the issue is a group::gitaly issue but a community contributor or
someone on another team ends up contributing to it.
|
|
Sometimes it's useful to quickly look up an object's info based on a
revision (which can include a path).
|
|
git-hash-object(1) does fsck on the objects its writing whereas
git-mktree(1) does not have the benefit of such safety mechanisms. Swap
to using git-hash-object(1) in our WriteTree() call.
|
|
housekeeping: Improvements for integration into the WAL
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5520
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
|
|
The test
TestServer_PostUploadPackSidechannel_gitConfigOptions/no_config_options/failing_request_because_of_hidden_ref_config
is flaky in nature. Let's skip it till we can find a fix for the race
condition.
|
|
The useless `repoPath` should be discovered earlier in merge request
https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5474, but
unfortunately we added a redundant param in `merge` method and no syntax
error is reported then.
Signed-off-by: Xing Xin <xingxin.xx@bytedance.com>
|
|
With the upcoming WAL (via TransactionManager), we need a way to inhibit
running and cancel ongoing calls to git-pack-refs(1), while the
TransactionManager is modifying refs.
This is because when git-pack-refs(1) is in progress, git would add
locks to the refs which could block the TransactionManager from
modifying the said refs. In short, the TransactionManager needs
exclusivity to the refs for modification.
To facilitate this, `packRefsIfNeeded` now only runs if there are no
inhibitors added. This is provided via the `repositoryStates` within the
`RepositoryManager`. The `repositoryStates` provides methods which allows
users to add inhibitors for `git-pack-refs(1)` too.
The provided methods are internal to the package and can be easily
exposed via the RepositoryManager when required.
|
|
What
---
Change the value of `run_cmds_in_cgroups` no by default
Why
---
We've been running cgroups in GitLab.com since [October
2022](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/344#note_1150885882)
and we haven't seen any fundamental problems with it. We've also
extended cgroups to [include
cpu_quota_us](https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5422).
In https://gitlab.com/gitlab-com/gl-infra/reliability/-/issues/23573 we
are noticing that some `git` commands aren't getting assigned to the
repository cgroup because sometimes feature flags aren't set and since
this is by default `false` we go to the `runsv` cgroup.
Reference: https://gitlab.com/gitlab-com/gl-infra/reliability/-/issues/23573#note_1379307222
Reference: https://gitlab.com/gitlab-org/gitaly/-/issues/4102
Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>
|
|
Improve concurrency limits and pack-objects logs
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5719
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Pavlo Strokov <pstrokov@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
|
|
Dealing with TreeEntry pointers is more convenient, since in a future
commit we will start to modify TreeEntry objects in place for various
operations. ListEntries() already returns a slice of TreeEntry pointers.
Change WriteTree to take a slice of TreeEntry pointers for the sake of
consistency.
|
|
ref: Delete DeleteRefsStructuredErrors feature flag
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5726
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
|
|
Now that this feature flag has been running in production for a while
now: https://gitlab.com/gitlab-org/gitaly/-/issues/4348, we can remove
it.
|
|
praefect: list-untracked-repositories sub-command migrated
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5689
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: Pavlo Strokov <pstrokov@gitlab.com>
|
|
|
|
|
|
Bump go to 1.19.9
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5727
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: Ash McKenzie <amckenzie@gitlab.com>
|
|
|
|
proto: Drop deprecated `http_host` fields
Closes #4502
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5722
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
Lately, the pack-objects RPC has been emitting logs that are difficult
to follow as they are each displayed on separate lines. This can be
problematic when attempting to investigate related issues. To address
this, a commit has been made to transfer these logs to gRPC logs. The
fields are stored in command.Stats, a FieldsProducer, and eventually
collated in a gRPC logging middleware.
|
|
There are two versions of concurrency limiting: a generic per-rpc option
and a pack-objects option. These exist on different layers, with the
former being at the middleware layer and the latter located before
calling the git-pack-objects command. It can be difficult to distinguish
between the two from the logs. To address this, a limiter type has been
added to the concurrency monitor through this commit.
|
|
We employ multiple levels of concurrency limitation and it is crucial to
have observability to comprehend how requests are being limited.
Although Gitaly has some useful logs, particularly about queuing states,
they are not integrated with the gRPC logs, making it difficult to
understand the situation. To address this, we have unified all
concurrency-limiting logs in one location through this commit.
Furthermore, we have added new logging concurrent-limiting fields to
enhance the logging capabilities.
|
|
|
|
|
|
ref: Remove deprecated FindAllBranchNames and FindAllTagNames RPCs
Closes #4658
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5723
Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
The 'list-untracked-repositories' sub-command migrated from the old
implementation to the new approach with usage of the
third party library. The invocation of the command remains
the same, but it has more descriptive representation when
help is invoked. Also, it is possible now to invoke help
only for the sub-command to see more details about it and
its usage.
Part of: #5001
|
|
We have deprecated the FindAllTagNames RPC in favor of ListRefs in
commit 55797560fc (proto: Deprecate FindAllBranchNames and
FindAllTagNames, 2022-12-07). As downstream users have been converted to
use the new RPC already we are thus safe to remove these RPCs.
Remove the FindAllTagNames RPC.
Changelog: removed
|
|
We have deprecated the FindAllBranchNames RPC in favor of ListRefs in
commit 55797560fc (proto: Deprecate FindAllBranchNames and
FindAllTagNames, 2022-12-07). As downstream users have been converted to
use the new RPC already we are thus safe to remove these RPCs.
Remove the FindAllBranchNames RPC.
Changelog: removed
|
|
The `http_host` field was added in order to avoid DNS rebinding attacks.
Callers would send us both the pre-resolved URL and the hostname so that
we would directly connect to the pre-resolved address, but still be able
to set the HTTP `Host` header as expected. This approach was replaced
in favor of the new `resolved_address` field, where Gitaly receives the
un-resolved URL and resolved IP address so that it can perform the
mapping internally.
We have thus deprecated the `http_host` field via cbdc529000 (proto:
Deprecate `http_host` field, 2022-09-30). Remove the field for the
FetchRemote RPC.
Changelog: removed
|
|
The `http_host` field was added in order to avoid DNS rebinding attacks.
Callers would send us both the pre-resolved URL and the hostname so that
we would directly connect to the pre-resolved address, but still be able
to set the HTTP `Host` header as expected. This approach was replaced
in favor of the new `resolved_address` field, where Gitaly receives the
un-resolved URL and resolved IP address so that it can perform the
mapping internally.
We have thus deprecated the `http_host` field via cbdc529000 (proto:
Deprecate `http_host` field, 2022-09-30). Remove the field for the
CreateRepositoryFromSnapshot RPC.
Changelog: removed
|
|
The `http_host` field was added in order to avoid DNS rebinding attacks.
Callers would send us both the pre-resolved URL and the hostname so that
we would directly connect to the pre-resolved address, but still be able
to set the HTTP `Host` header as expected. This approach was replaced
in favor of the new `resolved_address` field, where Gitaly receives the
un-resolved URL and resolved IP address so that it can perform the
mapping internally.
We have thus deprecated the `http_host` field via cbdc529000 (proto:
Deprecate `http_host` field, 2022-09-30). Remove the field for the
CreateRepositoryFromURL RPC.
Changelog: removed
|
|
The `http_host` field was added in order to avoid DNS rebinding attacks.
Callers would send us both the pre-resolved URL and the hostname so that
we would directly connect to the pre-resolved address, but still be able
to set the HTTP `Host` header as expected. This approach was replaced
in favor of the new `resolved_address` field, where Gitaly receives the
un-resolved URL and resolved IP address so that it can perform the
mapping internally.
We have thus deprecated the `http_host` field via cbdc529000 (proto:
Deprecate `http_host` field, 2022-09-30). Remove the field for the
UpdateRemoteMirror RPC.
Changelog: removed
|
|
The `http_host` field was added in order to avoid DNS rebinding attacks.
Callers would send us both the pre-resolved URL and the hostname so that
we would directly connect to the pre-resolved address, but still be able
to set the HTTP `Host` header as expected. This approach was replaced
in favor of the new `resolved_address` field, where Gitaly receives the
un-resolved URL and resolved IP address so that it can perform the
mapping internally.
We have thus deprecated the `http_host` field via cbdc529000 (proto:
Deprecate `http_host` field, 2022-09-30). Remove the field for the
FindRemoteRootRef RPC.
Changelog: removed
|
|
|
|
Remove uncessary fields from pack-objects cache key computation
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5716
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Approved-by: Christian Couder <chriscool@tuxfamily.org>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
|
