| Age | Commit message (Collapse) | Author |
|---|
|
This reverts commit 7de65fabe6eaf803771cf0dd5d53dbaf1e628d56, reversing
changes made to 0ffc495be9bb5a8f4ff60764b545443d54210e56.
|
|
Adds a handler to Praefect to intercept calls to the WalkRepos RPC. The
handler provides an alternate implementation of listing repositories in
a storage, which queries the Praefect DB rather than walking the
filesystem on disk. This is required so when the RPC is invoked via
Praefect, the DB is used as the source of truth rather than a random
Gitaly node.
The only user-facing difference between this and the original
implementation is that the `modification_time` attribute of the response
message is left empty, as this cannot be determined via the DB.
|
|
Now that we've adjusted the restore mechanism to delete individual
repos as needed, this RPC is no longer required. See the following
issues for more context:
- https://gitlab.com/gitlab-org/gitaly/-/issues/5357
- https://gitlab.com/gitlab-org/gitaly/-/issues/5269
Changelog: deprecated
|
|
For the `ProcReceiveHook` to work we need to create a RPC which is
called by the `gitaly-hooks` binary. Before we create this RPC, let's
note the definition of the RPC in the `hook.proto` file.
In the following commits we'll define the client and the service
following this definition.
|
|
Registers the gRPC reflection service with the Praefect server to enable
tools like grpcui to detect the server's endpoints. With this change, it
is possible to test Praefect's endpoints locally with the following:
grpcui -unix -plaintext <path/to/praefect.socket>
|
|
Enable transactions for object pools
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6562
Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: Sami Hiltunen <shiltunen@gitlab.com>
|
|
The `TransactionalAlternatesDisconnect` feature flag enables
transactions for the `DisconntectGitAlternates` RPC. Remove the feature
flag so that transactions are always enabled when client perform object
pool disconnection operations.
|
|
This commit enables transactions for LinkRepositoryToObjectPool.
This requires just recording the updated alternate link in the
transaction.
Transactions require the object pool and the member repository to
be in the same partition. This may not be the case when Praefect
attempts to reconciliate alternate link differences during
replication. Moving repositories between partitions is difficult, so
for now we'll simply ignore the pool linking failure in Praefect and
leave accept deviating alternate links.
|
|
Gitaly is using the repository's path to identify whether a repository
is a pool. Rails' pool paths are checked exhaustively with a regex and
Praefects paths are checked for a given prefix. These checks are not
sufficient to identify pools with transactions as the pool path will
be prefixed by the snapshot's root directory. Account for this by
changing the checks to verify the suffix of the path matches the
expected pool suffix.
A test was removed that is asserting behavior related to invalid pool
paths. Such state can't be created through the API anymore as we're
verifying the pool path in a stricter manner. We've never created
such paths as are being tested, so opted for removal rather than
maintaining the test.
|
|
There are 2 instances in our code base where we have two errors and want
to merge them. While the best fix would be to use `errors.Join()`, for
now, let's just fix the type so that the linter doesn't barf.
|
|
Similar to the previous commit, fix the remaining error comparisons
manually to use errors.Is().
|
|
Replace all io.EOF errors comparisons with errors.Is(). This ensures
that these errors work correctly with wrapped errors. This also makes
way for adding an error linter in the upcoming commits.
The changes in this commit were done automagically using project wide
replace methods provided by the text editor.
|
|
The Listen() function runs and endless loop. It always returns an
errors. With this regards, it makes no sense to check if the error is
nil. Remove this check.
|
|
The `ReplicateRepositoryObjectPool` feature flag enables object pool
replication through the `ReplicateRepository` RPC. Remove the feature
flag so clients can always request object pool replication.
|
|
Remove NamespaceService
Closes #3803
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6292
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Sami Hiltunen <shiltunen@gitlab.com>
|
|
proto: Add documentation for `RefService`
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6527
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
praefect: Reconcile non-matching object pools during replication
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6526
Merged-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
featureflag: Remove `TransactionalLinkRepository`
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6528
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
The `TransactionalLinkRepository` feature flag, when enabled, makes the
`LinkRepositoryToObjectPool` RPC transactional. Remove the feature flag
so that the RPC is always transactional for clients.
|
|
The `CreateBranch` RPC has been removed and there is no longer a need
for its associated request and response messages. Update call sites to
now use `UserCreateBranch` messages.
|
|
When a Praefect replica becomes out-of-date with the primary repository,
a replication job is scheduled to sync repository state. If the primary
and secondary replica are linked to non-matching object pools, the
replication job will always fail.
In this scenario, to reconcile the different object pools the target
repository should first disconnect from its object pool and then be
linked to the matching source repository object pool. Update Praefect
replication jobs to support synchronizing repositories with non-matching
object pools.
|
|
Remove the RenameRepository RPC as its no longer being used. With this
we delete all associated code and tests around this RPC and also delete
the proto definitions. While it might have been easier to review as
isolated commits, the code is highly intertwined making isolation
harder, as such the whole deletion is taken care of in this commit.
|
|
When a secondary repository replica in Praefect is out of sync with the
primary replica, Praefect creates a replication job that performs the
`ReplicateRepository` RPC to replicate the up-to-date primary onto the
secondary. As part of this operation the replicator also checks if the
source repository is linked to an object pool and if so links the target
to the matching object pool on the secondary.
If the primary is not linked to an object pool and the secondary is
linked to an object pool, no change is made to the target repository
object pool relationship and the replication job continues normally.
This is problematic because it allows secondary repositories to diverge
from the primary. Also if the repository attempts to link to a new
object pool via the `LinkRepositoryToObjectPool` RPC, the secondary
repositories will always fail due to already being linked to an object
pool.
To prevent this from occurring, the target repository should also
perform the `DisconnectGitAlternates` RPC if it is determined that the
source repository is not linked to an object pool. This change updates
the Praefect replicator to perform object pool disconnects to ensure the
target repository remains consistent with the source.
|
|
As part of the Praefect replication job, a target repository the object
pool link of the source repository. Add tests to verify that the
Praefect replicator replicates object pool links.
|
|
NamespaceService has been unused for a long time so let's remove it.
It was part of the hashed storage migration which was already finished
in GitLab 13.0.
Some tests were relying on the NamespaceService methods. Replace the
usage with storage scoped mutators or accessors from other services.
|
|
We have a test asserting a specific error is returned when attempting
to resolve a non-existent name. The 'not-existing.com' address used
in the test has since been registered and no longer results in a
resolving error. Remove the test to have the pipeline pass. We're
probably fine even if we don't test for this specific case.
|
|
The DatalossCheck RPC has been replaced by the Dataloss RPC. Let's
remove the code, the RPC definition, and associated messages.
|
|
TestProxyWrites is not creating the test repository in the Gitaly
storages. This causes the test to error out with a repositroy not
found error when running with WAL. Fix the test setup to actually
create the target repository in the Gitaly storages.
|
|
TestCoordinator_grpcErrorHandling is not creating the test repository
on the Gitaly storages configured as part of a virtual storage. This
causes the test to fail when WAL is enabled as the UserCreateBranch
errors with unexpected error due to the repository not existing. Setup
the repository in the storages to avoid this.
|
|
The ctxlogrus package is going away with the replacement being log
fields extracted from the context via `log.DebugContext()` et al.
Refactor the code to stop using ctxlogrus by injecting a logger and
using the new context-based logging methods.
|
|
The ctxlogrus package is going away with the replacement being log
fields extracted from the context via `log.DebugContext()` et al.
Refactor the code to stop using ctxlogrus by injecting a logger and
using the new context-based logging methods.
|
|
Merge the functionality that the field extractor provides into our
requestinfohandler logic. This unifies two related functionalities and
will eventually make it easier to move away from the tags mechanism that
is going away in go-grpc-middleware v2.
Add tests to make sure that the requestinfohandler interceptors work as
expected.
|
|
We have two different mechanisms to inject information about the current
gRPC call into the context:
- The metadatahandler injects data about the metadata, e.g. the peer
information and whether a call is an accessor or a mutator.
- The field extractor that gets set up via go-grpc-middleware's tags
interceptor and that injects data about the request.
The tagging mechanism used by the field extractor is going away in the
go-grpc-middleware dependency though without replacement. Furthermore,
both functionalities are related to each other and can thus easily share
the same infrastructure. We are thus about to merge them.
As a first step, let's rename the metadatahandler to requestinfohandler
such that it better reflects the new reponsibility when it will start to
handle both gRPC call metadata and request information.
|
|
In order to retain correlation IDs of replication jobs between the time
they are added to the database and the time when they are processed, we
store the correlation ID as part of the database entry. We store this
field generically in the `params` section, which is backend by a JSON
encoded column, where we use key-value pairs.
The way we use the key here is a bit of a mess though, as we reuse the
same key that our metadatahandler uses to inject the correlation ID for
logging purposes. It goes without saying though that it is not a good
idea to conflate a string for representation (logging) and for storing
data consistently (the replication parameters). While the key is in fact
the same right now, it could totally be that it might change for our
logging infrastructure at some point or even go away.
Separate the concerns by introducing a new constant in the `datastore`
package that holds the key we want to use in the database. When logging
data, we now consistently use the key from `correlation.FieldName`,
which is shared across GitLab. And for the database, we consistently use
the newly introduced constant.
|
|
We don't have any tests that verify that the sentryhandler interceptors
work as expected. With the current layout this is also quite impossible
to do, because the event that is to be reported will be passed to the
`sentry.CaptureEvent()` function without giving us a way to intercept
the captured event.
Refactor the code such that it becomes possible to replace the function
that reports events. Like that, we can intercept the capture function
and thus exercise the code more readily.
|
|
Inject a logger into the Praefect per-repository router and convert it
to use context-aware logging.
|
|
Inject a logger into the Praefect coordinator and convert it to use
context-aware logging.
|
|
Start using the new context-aware logging facilities in places where we
have a logger available.
|
|
Many callsites extract the logger via the passed-in context by using the
`ctxlogrus` package. The ability to add loggers to the context via this
package is going away upstream though in favor of a new set of logging
functions that accept a context as input parameter themselves.
We're thus going to introduce an equivalent interface, but that will
require our service servers to have a logger available to them. Prepare
for this change by injecting the logger into both Gitaly and Praefect
servers.
|
|
This reverts merge request !6408
|
|
log: Unify setup of the logging interceptors
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6408
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
Remove the `Errorf()` function from the `Logger` interface. Please refer
to the preceding commit that removes `Debugf()` for the motivation
behind this change.
|
|
Remove the `Warnf()` function from the `Logger` interface. Please refer
to the preceding commit that removes `Debugf()` for the motivation
behind this change.
|
|
Remove the `Infof()` function from the `Logger` interface. Please refer
to the preceding commit that removes `Debugf()` for the motivation
behind this change.
|
|
Next to `Debug()` and related functions our `Logger` interface also
provides `Debugf()` and related functions that takes a formatting string
as well as a set of arguments for it. There are multiple reasons though
why we don't want to expose these functions:
- They do not exist in the `slog` package, which thus causes our own
internal interface to diverge from our desired target solution.
- It is discouraged to put variable data into the message itself.
Instead, callsites are encouraged to add any variable data as
structured log data.
- It often gets abused in contexts where it's not required at all to
call the formatting variant as all we pass in is a static string.
- We have a pending addition of logging functions that also take a
context parameter as input. The formatting family would cause us
to require four more functions that take such a context as input.
Let's remove `Debugf()` in favor of `Debug()`. Most of the existing
callsites are converted to instead use structured logging data.
|
|
Constructing the logging interceptors is quite involved and requires the
caller to set up multiple different structures. Besides being hard to
understand, it also requires us to expose the fact that we're using the
logrus go-grpc-middleware to our callers.
Refactor the code to unify all of the plumbing into the logger itself.
All the caller needs to pass in now is any additional fields producers
they wish to install, but other than that things are self-contained. As
this change requires the need to pass in the logrus message producer, it
ensures that we can install non-logrus loggers as interceptors.
Note that this requires us to change some of our tests to also install
the `log.PerRPCLogHandler`. This is required because our own logging
interceptor plays some games with the log message such that it can also
include gRPC statistics and thus defers writing the log message to a
later point. This late rpoint is the per-RPC log handler, which thus
becomes mandatory in order to observe any log messages.
|
|
Inline the setup of the log message producer in the Praefect server
setup. This change makes a subsequent refactoring of how the logging
interceptors are constructed more straight-forward.
|
|
The `log.LogrusLogger` type directly wraps the `logrus.Entry`, thus
exposing all of its functions. We don't want anybody to call them in the
ideal case so that we can ensure that we use our own logging interface
almost exclusively throughout the codebase.
Let's hide the `logrus.Entry` by making it a private member of the
`log.LogrusLogger`. While this requires us to explicitly implement some
of the expected interface-level functions, by now it's really only four
of them and thus manageable.
There are still some very limited callsites in our tests that require
access to the `logrus.Entry`, which will be addressed in another
iteration. Meanwhile, we expose an explicit `LogrusEntry()` getter that
makes the entry accessible to those usecases.
|
|
While we already have a `service.Dependencies` type around for quite a
long time, we still pass in dependencies explicitly when constructing
the actual server. This makes it harder than necessary to make a server
require more dependencies as you will have to adjust all callsites where
the server is currently getting constructed.
Simplify the code to instead inject the `service.Dependencies` type into
the server directly. This will allow us to propagate dependencies more
readily in the future.
|
|
Remove some more uses of the `logrus` package in our codebase where it's
not strictly required.
|
