Skip to content
Snippets Groups Projects
This project is mirrored from https://gitlab.com/gitlab-org/gitaly.git. Pull mirroring updated .
  1. May 05, 2022
    • John Cai's avatar
      proto: Add DeleteRefsError structured error · f94fa279
      John Cai authored
      f94fa279
    • Patrick Steinhardt's avatar
      Merge branch 'pks-gitaly-internal-sockets-trim-max-length' into 'master' · 3f35f74c
      Patrick Steinhardt authored
      gitaly: Shorten maximum internal socket path length
      
      See merge request gitlab-org/gitaly!4523
      3f35f74c
    • Patrick Steinhardt's avatar
      Merge branch 'jc-remove-gomod-exclude-directive' into 'master' · 1f98d5a9
      Patrick Steinhardt authored
      go.mod: Remove exclude directive
      
      See merge request gitlab-org/gitaly!4525
      1f98d5a9
    • Patrick Steinhardt's avatar
      gitaly: Shorten maximum internal socket path length · b25bd0f0
      Patrick Steinhardt authored
      With the migration to the new runtime directory, Gitaly has moved its
      internal socket directory as well. Unfortunately, this change resulted
      in an expansion of the internal socket path length, and because Unix
      systems put a limit on Unix socket paths this has caused Gitaly to not
      come up on some systems anymore.
      
      This problem is entirely fixable by the administrator by configuring a
      different runtime directory path that has a shorter prefix. But we can
      at least try to shorten the paths we generate a bit. Right now, Gitaly
      creates three different types of internal sockets:
      
          - The internal socket that is used e.g. by `gitaly-hooks` to connect
            back to Gitaly. This file is created as `internal_${PID}.sock`.
            Including the PID is not required anymore though: the runtime
            directory always contains `gitaly-${PID}` anyway. So we can easily
            shorten this to just `intern`. This leaves us with a socket name
            length of 6.
      
          - The Ruby worker sockets, which are created as `ruby.$N`. `$N` is
            the number of any worker, and typically shouldn't be larger than
            in the tens. This leaves us with a typical socket name length of
            7.
      
          - The internal test socket that is created to verify whether we can
            create and connect to internal sockets. This is done as a sanity
            check to alert administrators early on in case the socket path
            length exceeds the system's limits. Right now it's created as
            `test-XXXX.sock`, but given that we're about to change the naming
            strategy we must only ensure that it's as long as the longest
            socket we're creating. Furthermore, we don't need to create it
            with a random part anymore given that the runtime directory is
            keyed by PID. We thus use `tsocket`, which has a socket name
            length of 7.
      
      With these changes in place we reduce the maximum socket name length
      from 19 characters to 7 characters, which gives us administrators 12
      characters more room to play with.
      b25bd0f0
    • Patrick Steinhardt's avatar
      Merge branch 'toon-danger-exclude-gen-proto' into 'master' · dd2e9041
      Patrick Steinhardt authored
      Danger: Exclude generated protobuf files in size
      
      See merge request gitlab-org/gitaly!4526
      dd2e9041
    • Patrick Steinhardt's avatar
      Merge branch 'jc-limithandler-return-structured-limits' into 'master' · 9179bbb4
      Patrick Steinhardt authored
      limithandler: Return structured errors
      
      Closes #4197
      
      See merge request gitlab-org/gitaly!4507
      9179bbb4
  2. May 04, 2022
  3. May 03, 2022
    • John Cai's avatar
      limithandler: Return structured errors · 18c5b92e
      John Cai authored
      In order to alert upstream clients that Gitaly has reached capacity,
      return a structured error of gitalypb.LimitError that clients like
      workhorse, gitlab-shell, and rails can parse and take action on.
      
      These errors all have the gRPC status Unavailable.
      
      Changelog: changed
      18c5b92e
    • John Cai's avatar
      Merge branch 'jc-cherry-pick-structured-errors-proto' into 'master' · 055d37fe
      John Cai authored
      proto: Add CherryPickError type for UserCherryPick structured errors
      
      See merge request gitlab-org/gitaly!4497
      055d37fe
    • John Cai's avatar
      proto: Add UserCherryPickError type for UserCherryPick structured errors · 44e98aba
      John Cai authored
      In UserCherryPick, there are some error conditions where instead of
      returning an error, we return OK and embed the error in the response.
      Instead of doing this, we want to return structured errors.
      
      This change adds the proto definition for this error.
      44e98aba
    • Sami Hiltunen's avatar
      Return correct error when repository has no up to date replicas · 7c78dcb8
      Sami Hiltunen authored
      GetConsistentStorages retrieves the up to date replicas of a repository.
      If the repository has no up to date replicas, it returns a repository
      not found error. This is incorrect as the repository may still exist
      from Praefect's point of view, it's just that the replicas with the
      latest changes have been lost. This commit changes GetConsistentStorages
      to return a more descriptive error for this scenario instead of a
      repository not found error.
      
      Changelog: fixed
      7c78dcb8
    • Sami Hiltunen's avatar
      Merge branch 'pks-check-objects-exist-improvements' into 'master' · a374f9bf
      Sami Hiltunen authored
      commit: Various fixes for `CheckObjectsExist()`
      
      See merge request gitlab-org/gitaly!4510
      a374f9bf
    • Sami Hiltunen's avatar
      Merge branch 'pks-remove-pack-objects-hook' into 'master' · 86c5a948
      Sami Hiltunen authored
      proto: Remove deprecated `PackObjectsHook()` RPC
      
      Closes #4138
      
      See merge request gitlab-org/gitaly!4508
      86c5a948
    • Patrick Steinhardt's avatar
      proto: Remove deprecated `PackObjectsHook()` RPC · c39a684b
      Patrick Steinhardt authored
      The `PackObjectsHook()` RPC is deprecated and its implementation has
      been removed in 473e3540 (Deprecate PackObjectsHook, 2021-10-25).
      Now that v15.0 is close, let's also remove the Protobuf definitions.
      
      Changelog: removed
      c39a684b
    • Patrick Steinhardt's avatar
      git: Disallow more whitespace characters in revisions · b78ec2da
      Patrick Steinhardt authored
      Revisions cannot contain any whitespace characters, and this is an
      assumption that we make in many parts of our application. Let's make the
      enforcement of this stricter by also verifying that there are no tabs or
      newlines in revision names.
      b78ec2da
    • Patrick Steinhardt's avatar
      git: Refactor revision-validation tests · 6ff4585b
      Patrick Steinhardt authored
      Refactor revision-validation tests to be more conformant with our
      current coding style. Furthermore, extend these tests to also verify
      that `ValidateRevisionAllowEmpty()` behaves as expected.
      6ff4585b
    • Patrick Steinhardt's avatar
      git: Rename file hosting revision verification logic · 8b41d770
      Patrick Steinhardt authored
      The logic to verify that revisions are valid revisions that can safely
      be passed to various Git components is hosted in a file `proto.go`. This
      is probably a historic artifact, but it doesn't make a lot of sense
      nowadays.
      
      Rename the file to `revision.go` to make it easier to find. While at it,
      this commit also moves related tests into `revision_test.go`.
      8b41d770
    • Patrick Steinhardt's avatar
      git: Move fallback time into `catfile` package · eef6255a
      Patrick Steinhardt authored
      When we fail to parse the commit time in the `catfile` package, then we
      instead return a fallback time. While the `catfile` package is the only
      user of this value, it is currently contained in the `git` package as a
      public variable.
      
      Move it over into the `catfile` package to keep it close to its user.
      eef6255a
    • Patrick Steinhardt's avatar
      commit: Avoid reallocating result array when sending object existence · bb1c2bab
      Patrick Steinhardt authored
      The sender we're using for chunked responses in `CheckObjectsExists()`
      is reallocating the result array every time we have sent out a message.
      This is needlessly wasteful though: we can just truncate the array and
      thus avoid a memory allocation. It's likely not going to matter much in
      this context, but the end result is easier to read, too.
      bb1c2bab
    • Patrick Steinhardt's avatar
      commit: Improve error messages in `CheckObjectsExist()` · a28046ec
      Patrick Steinhardt authored
      Most of the errors we return from `CheckObjectsExist()` are not
      annotated with either a proper error message nor with an error code.
      This makes it hard to find the location where the RPC fails if we see
      log messages.
      
      Improve this by adding proper error codes and messages.
      a28046ec
    • Patrick Steinhardt's avatar
      commit: Do not ignore revisions of the initial request · 6dc1d2b4
      Patrick Steinhardt authored
      The `CheckObjectsExist()` RPC is a streaming RPC: the caller may send
      one or more requests to check sets of revisions. This is done such that
      this RPC call also works in the context where millions of revisions need
      to be checked in a single call, which could otherwise exceed the gRPC
      memory limitations.
      
      The current behaviour is a tad weird though: revisions sent as part of
      the initial request are completely ignored. This doesn't feel right as
      the caller would now be forced to _always_ send at least two messages.
      It's also not documented to be the case, and last but not least all our
      other streaming RPCs don't do it this way either.
      
      So let's fix this behaviour and also honor revisions sent of the initial
      request.
      
      Changelog: fixed
      6dc1d2b4
    • Patrick Steinhardt's avatar
      commit: Fix verification of arguments in `CheckObjectsExist()` · 5fc7fa58
      Patrick Steinhardt authored
      The `CheckObjectsExist()` RPC doesn't verify it got a repository in its
      initial request. On the other hand, it only verifies that revisions are
      valid for the first request.
      
      Fix both issues.
      
      Changelog: fixed
      5fc7fa58
    • Patrick Steinhardt's avatar
      commit: Refactor `CheckObjectsExist()` tests to be more extensible · 4d075c78
      Patrick Steinhardt authored
      Tests for the `CheckObjectsExist()` are not quite extensible enough to
      test various error conditions, most importantly in the context of
      chunking. Refactor them.
      4d075c78
  4. May 02, 2022