| Age | Commit message (Collapse) | Author |
|---|
|
|
|
Git v2.20 introduces new Trace2 API. This API provides kinda hook to
extract the internal telemetry information of a Git process. It writes
the data to stderr or a file. The events are structured as a flat list
of consecutive events. When GIT_TRACE2_BRIEF variable is set, timing
information is omitted. That makes this format particularly hard to use
later.
In the foreseeable future, we use trace2 in two places: generating
equivalent distributed spans and expose pack object metrics. In the
future, we are looking forward to utilizing Trace2 more. As a result, we
should create an internal representation serving our purposes.
Naturally, the data can be presented as a tree.
This commit creates a parser that transform flat events to a tree data
structure.
For more information: https://git-scm.com/docs/api-trace2
|
|
Use %w when wrapping errors with context. Using %w has plenty of
benefits and allows unwrapping underlying error later. After this
feature was introduced, we favor error wrapping to string interpolation.
This commit implements a linter catching such offenses.
For more information
https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md#use-w-when-wrapping-errors
|
|
This commit implement a linter checking for the usage of '%s' and "%s".
Using manual quoting and bare string interpolation is not safe. We have
a style guide for it here:
https://gitlab.com/gitlab-org/gitaly/-/blob/master/STYLE.md#use-q-when-interpolating-strings
|
|
This commit adds the infrastructure to support custom linters.
Unfortunately, golangci-lint doesn't support dynamic linter. It means
that we need to compile each linter with `-buildmode=plugin` flag. The
compiled artifacts are then configured in `.golangci.yml` file. New
linter implementation follows the guide in this lint:
https://golangci-lint.run/contributing/new-linters/
Implementing this linter requires the following steps:
* Add analyzers implemting the checks to tools/gitaly-linters package
* Add the new checks to `lint.go` file
* Re-compile via `make gitaly-linters` or `make lint`
|
|
Fix quirky problems with tracing
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5406
Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
|
|
[ci skip]
|
|
Git Catfile cache is an optimization. It re-uses git-cat-file process if
it's already open by other goroutines. As a result, the process's life
span may be stretched outside the request starting the process. At
present, that span is attributed to the first request. That makes the
timing visualization of the whole trace looks weird. This commit
detaches that span from the starting request.
|
|
In the prior commit, some new utility methods were implemented to wrap
around tracing libraries. This commit replaces the existing calls to
opentracing library by new methods. Some some advanced usage still holds
references to opentracing, though.
|
|
Tracing is now powered by opentracing and labkit. They work fine, but
lack of some functionality, such as discarding all spans of path. In
addition, in the future, we may need to switch the tracing library. It
makes sense to implement a thin wrapper providing minimal interfaces.
This commit implements some foundation methods for this purpose.
|
|
go: Update module go.uber.org/goleak to v1.2.1
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5409
Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
go: Update module github.com/golang-jwt/jwt/v4 to v4.5.0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5405
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
go: Update github.com/ProtonMail/go-crypto digest to 7d5c6f0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5404
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
[ci skip]
|
|
'master'
gitaly: Delete log entry after being applied
Closes #4735
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5341
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Karthik Nayak <knayak@gitlab.com>
|
|
blob: Generate repository with LFS test data
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5402
Merged-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>
|
|
|
|
Now with the ability to generate test repositories with LFS pointers on
the fly, update the LFS tests to stop using seed repositories.
|
|
Currently tests in `lfs_pointers_test.go` rely on seeded repositories
for test data. This caused an issue when a seeded repository was updated
resulting in `TestListAllLFSPointers` consistently failing. This change
adds `setupWithLFS()` which configures a git repository with LFS
pointers for test data.
|
|
Add John Cai to support template
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5407
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Mark Wood <mwood@gitlab.com>
|
|
'4698-featureflag-enable-use-of-quarantined-repository-in-fetchsourcebranch' into 'master'
featureflag: Remove `FetchSourceBranchQuarantined`
Closes #4698
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5262
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
|
|
The flag `FetchSourceBranchQuarantined` is rolled out, now we can delete
the flag and modify the code/tests to remove the previous logic.
|
|
|
|
|
|
go: Update github.com/ProtonMail/go-crypto digest to 81033d7
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5394
Merged-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
middleware: Change method lookup warning log to debug
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5395
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>
|
|
hooks: Implement custom hooks replication
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5248
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>
|
|
Currently the `ReplicateRepository` RPC does not include the replication
of custom hooks for a repository. This change updates the RPC to include
replication of repository hooks stored in the `custom_hooks` directory
when the `gitaly_replicate_repository_hooks` feature flag is enabled.
The hooks replication is handled by `setCustomHooksTransaction()` from
the `SetCustomHooks` RPC which enables atomicity and transactionality.
|
|
Currently the `SetCustomHooks` RPC is set up in such a way that requires
a tar archive to be provided as input. In a future commit the
`ReplicateRepository` RPC will be updated to replicate hooks and
leverage the logic from `SetCustomHooks` to set hooks for the
repository. Since some repositories may not contain any custom hooks,
the logic needs to be updated to support this additional use case.
This change updates `extractHooks()` to be able to support receiving an
empty `io.Reader` as input without producing an error. Since the version
of tar used at runtime is system dependent, the code is generalized to
support both GNU and BSD tar.
|
|
Currently tests for the `SetCustomHooks` RPC rely on pregenerated tar
archives present in the `testdata/` directory. These archives can
instead be generated on the fly by reusing the tar archiving logic from
the `GetCustomHooks` RPC. This change factors out this logic into
`archive.Write()` which gets used by `GetCustomHooks` and several tests
to generate tar archives.
|
|
In `TransactionManager`, we don't Prune a log entry after it is
applied. This means the key lies in the database forever while providing
no additional value. Let's start pruning entries in the log after being
applied, we do this in `TransactionManager.applyLogEntry` wherein after
the transaction is applied we go ahead and delete the entry.
Since the entries are deleted, some of the tests need modification to
check the state of the database before deletion, modify all such tests.
|
|
The `hookContext` structure is provided to the handler of every
synchronization points. When we introduce the deletion of entries in the
write-ahead log, we want to check the state of the database and
pass/fail the test. To do this, we need the {database, test} structs.
Let's add them to the `hookContext`.
We override the default `testing.TB` object with our custom
`testingHook` struct, which overrides calls to `FailNow()` with calls to
`Fail()`, since the former can only be run in the main goroutine. This
allows us to now not worry about using `FailNow()` (this is the function
called by the `require` library).
|
|
The `TestTransactionManager` only adds hooks when it first creates the
manager. This means if the manager was created in step X, the hooks from
step X+1 won't be added unless the manages is stopped/started. To avoid
this issue, we create a new `applyHooks` function and call this at the
beginning of each step.
|
|
|
|
go: Update module github.com/jackc/pgx/v4 to v4.18.0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5390
Merged-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
remote: Support for SHA256 object format
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5368
Merged-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
tracker: Fix racey error insertion in test
Closes #4796
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5391
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Will Chandler <wchandler@gitlab.com>
|
|
Disable background verifier in tests
Closes #4773
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5399
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: Sami Hiltunen <shiltunen@gitlab.com>
|
|
git/stats: Fix race when verifying clone timings
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5401
Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
`TestErrorTracker_ClearErrors` would occasionally flake when the
insertion time of the second write error exactly matched `now`, causing
all write errors to be cleared.
Use the new `incr{Read,Write}ErrTime()` methods to ensure that the
errors times fall before or after our cutoff time.
|
|
The `Incr{Read,Write}ErrTime()` methods on `errorTracker` will use
`time.Now()` as the time of error. For production use this is a good
default and we should keep it, but when testing the `clear()` method
having a way to deterministicly set the time of error is useful. We have
experienced flakes when errors are inserted at _exactly_ the same time
as the cutoff.
Add internal methods `incr{Read,Write}ErrTime` that allow the caller to
specify the time of error. The public methods wrap around this with
`time.Now()` for convience.
|
|
This reverts commit ae63665f49c5acaf65bf847932843b583b485c4f.
|
|
Praefect's background verifier was recently changed to delete
invalid metadata by default. This can cause test flakes as some test
cases purposefully create such states. Given it would be surprising
to have the background verifier issue RPCs and delete state during
a test, disable it in the tests.
|
|
One of the things that the git/stats package does is to record timings
for various different steps in a fetch. As we cannot tell how long it
takes for each of these steps to complete the best we can do in the test
is to verify that timings are in the expected order.
The check we have for this is flaky though as it may happen that two
packets are observed in the same instant, but we test for times being in
strictly-increasing order:
=== FAIL: internal/git/stats TestClone (0.50s)
http_clone_test.go:73:
Error Trace: /builds/gitlab-org/gitaly/internal/git/stats/http_clone_test.go:73
Error: Should be true
Test: TestClone
Messages: post: expect time to receive first progress message (134.178921ms) to be greater than previous value 134.178921ms
Fix this flake by also allowing for the same time stamp.
|
|
housekeeping: Enable writing MIDX by default
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5397
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
localrepo: Allow unrelated histories in merge tree
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5385
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>
|
|
We have recently introduced the logic to write multi-pack indices during
repository maintenance. This new data structure allows Git to look up
objects more efficiently when there are many packfiles in a repository
by providing an index that spans across all the packfiles.
The code has been rolled out to production systems without any observed
issues. As this is a significant new data structure though we're playing
it safe and release the feature with its flag default enabled.
Changelog: changed
|
|
All RPCs of the remote service now support the SHA256 object format.
Adapt our tests to test with it.
|
|
The FindRemoteRepository RPC can be used to detect whether a repository
exists at the given URI. This is done by asking git-ls-remote(1) to
enumerate the remote's HEAD and then checking that we recognize it as a
valid reference.
This check breaks with the introduction of the SHA256 object format as
we explicitly verify that HEAD points to an object ID that is 40 hex
characters long. So when seeing a 64 hex characters long SHA256 hash
then we will wrongfully return that the repository does not exist.
Now this whole check feels kind of weird, and specifically verifying
specific object hash lengths isn't a great way to figure out whether a
repository exists or not. But for the time being we don't really have a
better way to figure this out, so our hands are kinda tied.
Refactor the RPC to recognize both SHA1 and SHA256 object hashes. While
this doesn't make the underlying issue any better, it at least continues
to work the sahe with repositories that use the SHA256 object format.
Changelog: added
|
