| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
|
|
|
|
[ci skip]
|
|
transaction: Drop fallback code to connect to Praefect via server info
See merge request gitlab-org/gitaly!3585
|
|
Refactor blob service to use Go pipelines for LFS objects
See merge request gitlab-org/gitaly!3584
|
|
Now that the pipelining code is in place, convert `ListLFSPointers()` to
use the new pipeline. The change is currently implemented behind a
feature flag to catch any issues with it, but eventually we can get rid
of the old implementation and only retain the new and much more flexible
pipeline architecture.
|
|
We're about to add a second implementation to the current existing
`ListLFSPointers()` implementation. Given that both callsites will
require validation of specified revisions, let's move it into
`ListLFSPointers()` directly such that the code is shared. Given that no
other callers of `findLFSPointersByRevision()` exist, this doesn't relax
any semantics.
|
|
Now that we have the infrastructure in place to set up a complete
pipeline which reads a set of revisions, extracts information about each
of the objects and then finally reads the objects from disk, this commit
adds a bunch of tests which do end-to-end tests of the assembled
pipeline.
|
|
While we now have a fully functional pipeline in place, it's not yet
able to filter down objects based on the information we got. This is now
implemented via a set of filters which can be inserted as intermediate
steps to reduce the number of objects passed onto the next stage.
|
|
In order to unify infrastructure when filtering down a list of revisions
to a list of objects, we're creating a set of pipeline steps which first
enumerate all objects, then extract information about these objects and
finally read the whole objects. This pipeline will allow us to flexibly
put together steps and filter down the object set at intermediate steps.
This commit implements the final pipeline step, which receives as input
a channel of object infos and reads each of the associated objects
fully via `git cat-file --batch`.
|
|
In order to unify infrastructure when filtering down a list of revisions
to a list of objects, we're creating a set of pipeline steps which first
enumerate all objects, then extract information about these objects and
finally read the whole objects. This pipeline will allow us to flexibly
put together steps and filter down the object set at intermediate steps.
This commit implements the second pipeline step, which receives as input
a channel of object IDs and outputs information about each of the
objects via `git cat-file --batch-check`.
|
|
In order to unify infrastructure when filtering down a list of revisions
to a list of objects, we're creating a set of pipeline steps which first
enumerate all objects, then extract information about these objects and
finally read the whole objects. This pipeline will allow us to flexibly
put together steps and filter down the object set at intermediate steps.
This commit implements the first pipeline step, which enumerates all
objects reachable from a set of revisions via git-rev-list(1).
|
|
Givven that all transactional voting nowadays happens via the
backchannel ID, there is no need anymore for the old Praefect server
structure. Let's delete it.
|
|
We do not use the Praefect server structure for anything anymore. Let's
strip it out from our tests such that we can remove it altogether.
|
|
Given that all transactional voting happens via the backchannel now,
there is no need to inject the Praefect server info anymore. Let's drop
it.
|
|
Now that all transactional voting uses the backchannel to connect back
to Praefect, we do not need the Praefect server info anymore. Let's drop
it.
|
|
Now that we only use the backchannel to connect back to Praefect for
transactional voting, there is no need to inject Praefect server info
into the Ruby sidecar anymore. Let's drop it.
|
|
We do not need the Praefect server info in any of the transactional
voting functions anymore given that we now always use the backchannel,
and the backchannel ID is redundantly encoded both in the Praefect
server structure and in the transaction information. Drop the Praefect
server parameter from voting functions.
|
|
Given that the backchannel voting code has been unconditionally enabled
in v13.12, we can now drop the old fallback code which was using the
Praefect server info if no backchannel was set.
|
|
The `CloneFromPoolInternal()` RPC is responsible for first creating the
desired target repo by closing from a pool repository and then updating
the pool via a fetch from the source repo. While the former step happens
via a direct call to git-clone(1), we connect to the remote service's
`FetchInternalRemote()` RPC for the latter.
This second step is problematic in the context of transactions though:
we call `FetchInternalRemote()` with the incoming context having been
converted to an outgoing context. The outgoing context thus retains
information about any ongoing transactions and also about the
backchannel ID. Given that this is a mutating RPC, the remote side will
now try vote on the transaction and dial back via the backchannel, but
given that the backchannel ID referred to the original Praefect node and
not to the now-intermediate Gitaly node, this cannot work and will thus
fail.
Now that we have `FetchInternalRemote()` exposed directly without having
to connect to the remote service, we can now easily fix the issue by
just calling this function directly.
No new tests are added because this failure will be exposed by
subsequent commits which remove the `PraefetcServer` structure.
|
|
The `git.WithRefTxHook()` nowadays only requires a `repository.GitRepo`
structure as input, but we still perform a cast to `gitalypb.Repository`
in `cloneFromPool()`. Drop this useless cast and pass in the repo
directly.
|
|
We'll need to be able to call `FetchInternalRemote()` on the same Gitaly
node without creating a connection to the remote service. Split out the
logic of `FetchInternalRemote()` into a separate non-receiver function
to allow for this.
|
|
We'll need to call `getRemoteDefaultBranch()` from a context where we
don't have a remote server as receiver. Pass in the connection pool
expicitly and make it a normal function to prepare for this.
|
|
Instead of having to pass around the command factory, we nowadays often
use the `git.RepositoryExecutor` interface which gets implemented by our
localrepo structure. Let's refactor `SetDefaultBranchRef()` to use this
interface.
|
|
blackbox: Code cleanups for blackbox and related stats package
See merge request gitlab-org/gitaly!3578
|
|
The "analyzehttp.go" file's most prominent member is the `Clone`
structure. Let's rename the file to "clone.go" to better match its
contents. This requires us to fix one last missing comment on the
`Clone` structure.
|
|
The `Clone` structure currently holds both parameters and results. Such
usage is always a code smell as it conflates two different concerns and
makes it harder to use correctly for the caller.
Let's disentangle both concerns by instead accepting parameters as part
of a refactored `PerformClone()` function. This function is not a
receiver anymore, but instead returns a newly constructed `Clone`
structure which only contains results.
|
|
The `Post` structure hosts statistics about a POST request against
git-upload-pack(1). Without knowing details about how git-upload-pack(1)
works though, one wouldn't be able to tell what it actually represents:
everything related to fetching the packfile.
Let's rename the structure to `HTTPFetchPack`, which more clearly labels
what it actually is about. While at it, this also refactors the way it's
constructed: instead of being constructed via the `Clone` structure,
it's now a standalone structure and can be constructed via
`performFetchPack()`. This disentangles concerns and makes it
easier to reason about the code.
|
|
The `Get` structure hosts statistics about a GET request against
git-upload-pack(1). Without knowing details about how git-upload-pack(1)
works though, one wouldn't be able to tell what it actually represents:
everything related to the reference discovery.
Let's rename the structure to `HTTPReferenceDiscovery`, which more
clearly labels what it actually is about. While at it, this also
refactors the way it's constructed: instead of being constructed via the
`Clone` structure, it's now a standalone structure and can be
constructed via `performReferenceDiscovery()`. This disentangles
concerns and makes it easier to reason about the code.
|
|
When doing a POST to git-upload-pack(1), then we send along a set of
"want"s which tell the remote side which references we want to receive.
So while this depends on us knowing which references exist in the first
place, it still is inarguably a property of the POST and not of the GET
which discovers available refs.
Let's make the relation more explicit by 1. moving wants into the `Post`
structure and 2. accepting announced references as parameter when doing
the post.
|
|
Right now, it's really hard to reason about which functions on the
`Clone`, `Get` and `Post` structures come from where given that we use
inlined structures all the time. To clean this up a bit and make code
easier to follow, this commit de-inlines all embedded structures into
proper member variables.
|
|
The code to parse the git-fetch-pack(1) body is tightly coupled with the
code which drives the actual request. This makes it hard to reason about
both concerns and also makes it hard to just parse a fetch pack body
without doing the actual request.
Refactor the code and split out a new `FetchPack` structure which
contains all logic to parse from an `io.Reader` without having to care
about the request itself. This structure is the equivalent to the
already existing `ReferenceDiscovery` structure.
|
|
Now that `printInteractive()` doesn't append a newline anymore, we can
convert other callers which up to now had to manually check whether
the `interactive` flag was set.
|
|
When `printInteractive()` is called, the assumption is that all lines
should be terminated by a newline. While that's true right now, this
assumption keeps us from using this function in other places which thus
need to manually check whether the `interactive` flag is set.
Do not append the newline in `printInteractive()` and adjust callers
accordingly to prepare for wider use of that function.
|
|
The gitaly-debug executable can be used to clone from a remote
repository and print several statistics about the clone itself, like for
example how long several stages took. Because of this, the stats package
supports an "interactive" flag which causes it to print various events
to stdout: only if it's set to true will `printInteractive()` actually
print anything.
One code smell we have right now is that `printInteractive()` returns an
error, but it's never actually checked. And we probably don't even want
to: it's only used for gitaly-debug to print to stdout, so it's harmless
enough if it would fail.
Get rid of the error code and just ignore when `fmt.Printf()` fails and
remove the golang-ci exemption.
|
|
Now that we have a Blackbox structure which can host all dependencies,
let's move Prometheus metrics into this structure to get rid of one more
source of globals.
|
|
Right now, the implenetation of Blackbox logic always gets a
configuration passed in as it is not associated to any structure. This
shortcoming keeps us from having all logic like for example also the
Prometheus counters as part of the struct, which means that they
currently live as globals.
Let's prepare to get rid of these globals by creating a new `Blackbox`
structure and implement blackbox logic as receiver functions. For now,
the struct only hosts the configuration.
|
|
We typically pass configuration structures around as a value instead of
as a pointer to make the configuration immutable to the receiving side.
One exemption to thi sis the blackbox code, where we pass it around as a
pointer.
Bring the code more in line with our own best practices and pass it as
simple value.
|
|
We have two representations of the sleep duration in the blackbox
Config: once a normal integer which gets parsed from the config file,
and once the converted `time.Duration`. Ideally, we'd have used a
`time.Duration` from the inception such that it's possible to specify
human-readable durations in the config file and not require any
conversion, but that train has departed by now given that the
gitaly-blackbox is already in use.
While we cannot merge those two variables now, we can at least make the
`SleepDuration` private to the package as it's not used anywhere else.
|
|
The blackbox code is lacking some documentation on public symbols. Add
it as required and remove the golang-ci exemptions.
|
|
featureflag: Remove reference transactions feature flag
See merge request gitlab-org/gitaly!3575
|
|
Reference transactions have now been baking for several releases and is
performing well. With backchannel voting having landed in release 13.12,
the last blocker which has kept customers from deploying these has been
removed. As a result, we can now finally drop the feature flag
altogether and always enable reference transactions.
Changelog: changed
|
|
Enable tx_config and tx_remote feature flags by default
See merge request gitlab-org/gitaly!3572
|
|
Upgrade protoc, protobuf, gRPC, Go and gRPC protoc plugins
See merge request gitlab-org/gitaly!3403
|
|
Changelog: changed
|
|
|
|
|
