James Liu (73e70ae2) at 18 Mar 00:25

... and 1 more commit

James Liu (e8572f8d) at 18 Mar 00:01

@eread it seems like these might have some value in centralising links that could change over time, so that only the single link needs to be updated. Do you think we should keep them around? I don't mind either way, but I thought you might have some opinions about it!

TIL about these footer-style link things. I've never used these in Markdown before.

Sorry @timofurrer! I don't have capacity at the moment

Thanks @jcaigitlab, LGTM! I'm not sure what the bot is complaining about here, since the format you have is what I've always seen in the past.

What does this MR do and why?

Praefect configuration: Enable TLS + DNS

A recent addition enables dns + tls. Document how this works and how to use it.

References

• !218360
• gitaly!8372
• gitaly!8522

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

@jcaigitlab @divya_gitlab I think this is ready for another look!

As mentioned here, I've had to change the implementation to call git-diff --numstat which incurs more latency overhead. Even with this overhead, I think there's a net reduction in latency as far as the user is concerned, since Rails no longer needs to make another DiffStats call after CommitDiff.

Let me know what you think, and if you're comfortable with the latency increase.

@jcaigitlab cool thanks!

Also, the diagram has a step Git writes refs. Should there be a separate step where Git is writing the objects? Does that come before or after the reference-transaction hook?

So the practical difference would be pipelines and such would take slightly longer to get triggered.

Makes sense! Do you know what would happen if the secondaries were lagging behind in generation number?

@patrickbajao I'm honestly not sure. I've only worked very sparingly on the Gitaly client code in Ruby.

It looks like the limitation was introduced in gitlab!136294 (merged) and specifically designed to stop Gitaly requests from being made within the async HTTP client. I don't know if the limitation is supposed to apply universally.

If I've read the code properly:

1. When the Gitaly client is invoked, it checks if the concurrency limitation is in effect.
2. The restriction is enabled by calling a helper function here, and the only caller seems to be the HTTP client.

Does this mean that the concurrency limitation is basically a no-op everywhere else?

Is this cost calculation something that already exists?

This link is for Gitaly on Kubernetes 😂

Did you mean to add #7097 instead?

I think we might need to include other clients like Workhorse and GitLab Shell. Most of the heaviest requests like fetches and pushes wouldn't go through Rails at all.

I like the idea of exposing a Gitaly pressure metric to (internal) clients as a means of encouraging them to throttle requests. Would the Gitaly team be responsible for implementing the client-side code too?

X-Score header

I couldn't find any information about this header. How is it supposed to work with Cloudflare?

I noticed that several issues were created recently around load monitoring and shedding. Should we link this document back to the work items?

There are many components that are changing, so it would be good for this to serve as an overview of what each component is, and the issue that is implementing the component.

The 2 second interval suggests the BurdenTracker is only really useful for long-running commands. For short-lived commands, we may not even get the chance to scrape /proc/<pid>/stat and /proc/<pid>/status before the command exits. Should we mention that here?

I'm very confused with how http.proactiveAuth is meant to work within Gitaly. I think it worked in the Runner because it already configures a Git credentials helper. The helper injects $CI_JOB_TOKEN making it implicitly available to Git when http.proactiveAuth is used.

We don't do anything with credential helpers in Gitaly, so I see only a few options where this would work:

• If the URL directly contains the username and password, like in this block of code.
• If auth headers were provided by Rails, which seems to happen in cases like Geo replication or when repositories are imported.

Should we enable http.proactiveAuth in the case when auth headers are provided too (in the block below)?

---

Another case would be that if FetchRemote is called against a public project, and the caller wouldn't supply any auth information even if they had the capacity to. Those requests would then contribute to the unauthenticated rate limit.

@oli.campeau @echui-gitlab mind having a look? Thanks!

The logic in the Rails API makes the scenario you described impossible. The comment also describes the intended behaviour with 0 upstreams.

James Liu (9b557947) at 16 Mar 06:19

... and 1 more commit