Bundle generation manager interface leaking implementation
This issue tracks some questions I have over wiring-up the bundle URI feature; mostly about the configuration of the bundleuri.GenerationManager
and the interface it exposes.
Question 1
The constructor of GenerationManager
takes a threshold
and a concurrencyLimit
as parameters: https://gitlab.com/gitlab-org/gitaly/-/blob/master/internal/bundleuri/manager.go?ref_type=heads#L45
Where do we take those configurations?
- Should they be in the Gitaly config file?
- Should we hardcode them (this would be ineffective because we would need to rebuild the application everytime we want to try out a new config, but it's the easiest way to get started with)
- Should we dynamically set them based on some algorithms, thus re-creating a manager for every request or so?
Question 2
The current strategy of the manager to determine when to create a bundle is based on the threshold and concurrency limit. But this strategy might change in the future, especially when replication will be rolled out (with Raft), and the same repo will now be replicated on different node. At that point, the Gitaly itself won't have all the information in a nutshell to make a decision about creating a bundle with the current strategy. A new one might be needed.
With the current state of things, isn't the interface leaking implementation?
The name of the function (GenerateIfAboveThreshold
) says a lot about the strategy used.
Shouldn't the GenerationManager
expose a more generic interface:
type BundleGenerator interface{
Generate(ctx context.Context, repo *localrepo.Repo)
}
Currently, the bundle manager is used directly, without an interface. I suggest to add one.