| Age | Commit message (Collapse) | Author |
|---|
|
When `TEST_OPTIONS` is specified as an argument to `make`, any options
specified in the `Makefile` will be overwritten with the user-provided
value. In several targets we append arguments to `TEST_OPTIONS`, making
them vulnerable to being squashed inadvertently.
For example, if running `make race-go TEST_OPTIONS='-run=<TEST>
-count=20'` to reproduce a race condition, the intended command would
be:
gotestsum --format short -- -run <TEST> -count 20 -race ./...
But because `TEST_OPTIONS` is specified, this is actually executed as:
gotestsum --format short -- -run <TEST> -count 20 ./...
Resolve this by using the `override` directive in targets that attempt
to append to `TEST_OPTIONS`. This will allow the user to add custom
options without squashing the target's additions.
|
|
'4455-feature-flag-roll-out-user_create_branch_structured_errors' into 'master'
operations: Structured errors in UserCreateBranch
Closes #4455
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4883
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: Karthik Nayak <knayak@gitlab.com>
|
|
Revert "Merge branch...
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4888
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: karthik nayak <knayak@gitlab.com>
|
|
objectpool: Always prune refs on fetches to fix D/F conflicts
Closes #4394
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4885
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
|
|
With 018958fb1 (objectpool: Fix conflicting references when fetching
into pools, 2022-07-27) we have started to prune references in object
pools that have been removed in their primary pool member. This is a
necessity to avoid conflicting references in the object pool and its
member, which would otherwise break updating such pools completely.
This code has been rolled out to production on August 11th without any
observed negative fallout, and the flag was subsequently enabled by
default. Remove the feature flag guarding it.
Changelog: changed
|
|
golangci-lint: Stop excluding many `Close` and `Serve` functions from errcheck linter
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4810
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
|
|
'4452-feature-flag-roll-out-simplify_find_local_branches_response' into 'master'"
This reverts merge request !4884
|
|
We don't properly verify the error code when closing the file that
corresponds to the packed binary. While this doesn't really matter in
the first place, fixing this one case lets us drop another exception to
our linting rules.
|
|
Many of our tests don't properly verify the error returned by the server
factory's `Serve()` function. Fix this shortcoming, which surfaces that
we mistakenly close some listeners manually that should indeed be closed
by gRPC itself. Drop the corresponding linting exception.
|
|
The `http.Server.Serve()` function will return an error when the HTTP
server has either failed unexpectedly or in case it wasn't properly shut
down. We don't check this error in many places though, which both causes
the errcheck linter to complain and hides issues we have with some of
our test setups.
Fix the missing error checks via a new `testhelper.MustServe()` helper
function that does the error checking for us. Drop the corresponding
linting exceptions.
|
|
The `grpc.Server.Serve()` function will return an error when the gRPC
server has either failed unexpectedly or in case it wasn't properly shut
down. We don't check this error in many places though, which both causes
the errcheck linter to complain and hides issues we have with some of
our test setups.
Fix the missing error checks via a new `testhelper.MustServe()` helper
function that does the error checking for us. This surfaces some test
errors for tests that forget to shut down the server or that manually
close the listener, which should be handled by gRPC itself. All of those
errors are fixed while at it.
Drop the corresponding linting exceptions.
|
|
We don't properly check the error when closing the `SidechannelWaiter`
in many places. Fix this and drop the corresponding linting exceptions.
|
|
The `close()` function we return as part of the server handshake is
responsible for closing all associated resources. In case closing any of
them fails though we never bubble up errors at all, but instead silently
succeed.
Fix this and return the first error we encounter. As we now correctly
check returned errors the errcheck linter is happy and we can drop
another exclude for the `yamux.Session` type.
|
|
We already check the error when we manually close the zlib writer, but
don't in the deferred call. This is fine though at it won't do anything
in the happy path as the writer was closed already, and we already got
an error in the unhappy path.
Explicitly ignore the error to make our linters happy.
|
|
While we check error codes returned by the tar writer in the general
case when writing an archive, we ignore it in case we have already seen
an error in `TarBuilder.Close()`. We didn't explicitly ignore the error
with a blanket-assignment though, and thus the errcheck linter
complains.
Fix this by assigning the error to the blanket variable and drop the
corresponding exclude from our linting rules.
|
|
Check error codes when closing a `safe.LockingFileWriter` or
`safe.Writer` and drop the corresponding exclude from our linting rules.
|
|
Check the error code when closing the Unix socket listener and drop the
corresponding exclude from our linting rules.
|
|
The errcheck linter is tasked with finding cases where we are mistakenly
not checking errors returned by a function. While we have it globally
enabled, one gap that we currently have is that we exclude all function
calls to `Close` and `Serve`. This exception is something we added back
when we added this linter to our rules as it has proven to be too much
work to fix all violations in the code base. Not checking any such
errors may easily hide issues we're not aware of though and is thus bad
practice.
Convert the blanket-exclude for all functions to instead explicitly
exclude all functions that currently violate it. Like this, it becomes a
lot more manageable to fix missing error checks one function at a time
and makes sure that we don't add new violations to our codebase that are
not covered by any of these explicit function excludes.
|
|
'4452-feature-flag-roll-out-simplify_find_local_branches_response' into 'master'
ff: set SimplifyFindLocalBranchesResponse to true
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4884
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: Karthik Nayak <knayak@gitlab.com>
|
|
With 2e642ccd (branches: Use 'UserCreateBranchError' behind a flag) we
have started using structured errors for the UserCreateBranch RPC
instead of relying on `Internal` errors.
This code was rolled out to production and the feature flag is currently
enabled by default. Let's remove the feature flag guarding it.
Changelog: changed
|
|
ref: Always use structured errors in FindTag RPC
Closes #4398
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4881
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
Since this featureflag has a rails/gitlab counterpart we cannot simply
delete it. We need to ensure that both Gitaly and rails have it set to
true by default for one release and then we can delete the feature flag
in the next release.
|
|
With 453ac57fc (ref: Fix `Internal` errors in `FindTag()` when tag
doesn't exist, 2022-08-01) we have adapted the FindTag RPC to return a
`NotFound` error instead of an `Internal` one when the tag wasn't found
in order to clearly label this error condition as expected.
This code has been rolled out to production on August 2nd without any
observed negative fallout. Remove the feature flag guarding it.
Changelog: changed
|
|
Makefile: Update Delve
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4879
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: Toon Claes <toon@gitlab.com>
|
|
The previous version did not work with Go 1.19, so update it to the
latest version.
|
|
[ci skip]
|
|
Update pg_query to v2.1.4
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4872
Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: John Cai <jcai@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Co-authored-by: Stan Hu <stanhu@gmail.com>
|
|
featureflag: Delete flag `GitV2371Gl1`
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4874
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
|
|
go: Update module github.com/prometheus/client_golang to v1.13.0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4814
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
go: Update module go.uber.org/goleak to v1.2.0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4868
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
go: Update module github.com/golang-jwt/jwt/v4 to v4.4.2
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4840
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
The feature flag `GitV2371Gl1` is completely rolled out in production.
Now lets delete the flag. With this change we can follow up with a bump
to the minimum version of Git used in Gitlay.
|
|
This fixes an issue with compiling on macOS with XCode 14
(https://github.com/pganalyze/pg_query/pull/256) among other bugs.
- Diff: https://my.diffend.io/gems/pg_query/2.1.3/2.1.4
- Log: https://github.com/pganalyze/pg_query/blob/main/CHANGELOG.md
This mirrors the same change in GitLab Rails:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98274
Changelog: changed
|
|
featureflag: Ensure to only contain valid characters
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4793
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: blanet <moweng.xx@alibaba-inc.com>
|
|
metrics: Remove Praefect read-only metric
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4858
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
go: Update module github.com/jackc/pgx/v4 to v4.17.2
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4865
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
praefect: Read line-by-line in track-repositories
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4855
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Will Chandler <wchandler@gitlab.com>
|
|
|
|
|
|
|
|
go: Update module github.com/pelletier/go-toml/v2 to v2.0.5
See merge request gitlab-org/gitaly!4856
|
|
Add #gitaly-alerts to team member onboarding template
See merge request gitlab-org/gitaly!4859
|
|
|
|
Log only mode for new repository calculation methods
Closes #4447
See merge request gitlab-org/gitaly!4848
|
|
Use the disk usage formatting directive to calculate the repository size
using cat-file. Also change the logging statement to log the bytes,
rather than kilobytes for the sake of clarity.
|
|
go: Update module google.golang.org/grpc to v1.49.0
See merge request gitlab-org/gitaly!4853
|
|
go: Update github.com/ProtonMail/go-crypto digest to 4b6e5c5
See merge request gitlab-org/gitaly!4847
|
|
blob: Standardize returned errors
See merge request gitlab-org/gitaly!4849
|
|
We should use %w for the error wrapping. That will
allow us to unwrap the error without loosing any context.
It is not use right now, but maybe in the future if we need
to change response based on the returned error in the chain
of interceptors.
And one simplification from fmt.Errorf() to errors.New().
|
|
In some cases error returned by RPC methods are prefixed with
its names. This is redundant as the caller knows exactly what
method is called and also the log entry always contains full
method name including service name. Instead, the prefixes for
errors in some cases were added to better describe what operation
failed. It should help to better navigate to the reason of the
error.
|
