Add a blocking container resolver

[achilleas-k]

This is a follow-up to #1153 and part of a bigger refactor meant to distribute some of the resolver conveniences from the new manifestgen package to the packages that implement the resolvers themselves.

This PR converts the container.Resolver from a concrete type to an interface with two implementations:

• asyncResolver: the old resolver that resolves container refs asynchronously and blocks on Finish().
• blockingResolver: a new resolver that resolves container refs synchronously, blocking on each call to Add().

The blockingResolver implements the Resolver interface but resolves containers synchronously, blocking execution on the resolve process inside the Add() method. This will become the only implementation in the near future. For now we will support both to make the transition easier for osbuild-composer. This change is made so that the container resolver behaves in the same way as the other content resolvers (packages and ostree commits) for consistency.

[supakeen]

I'll need to go through the code in detail (it looks fine on a first glance through it!) but I do have a question. The PR says that we're migrating the container resolver to its blocking implementation due to it being consistent with the other resolvers.

I'm wondering; do we lose anything from doing so? What are the implications if I want to resolve a large amount of containers (where I assume this to be the most visible)?

[achilleas-k]

I'll need to go through the code in detail (it looks fine on a first glance through it!) but I do have a question. The PR says that we're migrating the container resolver to its blocking implementation due to it being consistent with the other resolvers.

I'm wondering; do we lose anything from doing so? What are the implications if I want to resolve a large amount of containers (where I assume this to be the most visible)?

All uses of the container resolver end up adding all containers to the queue then immediately calling Finish(), which makes the whole thing roughly equivalent to the new blocking resolver.

The differences are that the blocking resolver will block on Add(), resolving each container as it's added. In the new implementation, this will done by calling the Resolve() function, which makes it more clear that this will happen.

There are scenarios where the two aren't equivalent, of course. If we need to resolve a lot of containers and only one is particularly slow, the async version will resolve all the other ones while the slow one waits.

In osbuild-composer, the container resolve job runs on a worker, so the whole thing runs async from the rest of the manifest generation process, but internally, each container will now be resolved in sequence.

The main purpose here is consistency though. Having a common interface (as much as possible) between our three types of resolvers. It might be worth considering making them all async, but that's probably a bit of a wasted optimisation:

• Depsolving multiple package set chains in parallel will probably end up blocking on the rpmmd cache lock, so we wont get anything out of that.
• For ostree commits, we only ever need to resolve one, either the parent of the new commit we're building, or the commit we're going to use to build an ISO or a disk image. Theoretically, we could have multiple commits to resolve, but practically this scenario never came up.
• Containers are the only thing we might have to resolve multiples of and could do in parallel, but realistically we only ever need to do a handful.

If you think it's better to do containers async, I wont disagree, but I would probably change the interface anyway to match the other resolvers.

[supakeen]

I'll need to go through the code in detail (it looks fine on a first glance through it!) but I do have a question. The PR says that we're migrating the container resolver to its blocking implementation due to it being consistent with the other resolvers.
I'm wondering; do we lose anything from doing so? What are the implications if I want to resolve a large amount of containers (where I assume this to be the most visible)?

All uses of the container resolver end up adding all containers to the queue then immediately calling Finish(), which makes the whole thing roughly equivalent to the new blocking resolver.

The differences are that the blocking resolver will block on Add(), resolving each container as it's added. In the new implementation, this will done by calling the Resolve() function, which makes it more clear that this will happen.

There are scenarios where the two aren't equivalent, of course. If we need to resolve a lot of containers and only one is particularly slow, the async version will resolve all the other ones while the slow one waits.

In osbuild-composer, the container resolve job runs on a worker, so the whole thing runs async from the rest of the manifest generation process, but internally, each container will now be resolved in sequence.

The main purpose here is consistency though. Having a common interface (as much as possible) between our three types of resolvers. It might be worth considering making them all async, but that's probably a bit of a wasted optimisation:

* Depsolving multiple package set chains in parallel will probably end up blocking on the rpmmd cache lock, so we wont get anything out of that.

* For ostree commits, we only ever need to resolve one, either the parent of the new commit we're building, or the commit we're going to use to build an ISO or a disk image.  Theoretically, we could have multiple commits to resolve, but practically this scenario never came up.

* Containers are the only thing we might have to resolve multiples of and could do in parallel, but realistically we only ever need to do a handful.

If you think it's better to do containers async, I wont disagree, but I would probably change the interface anyway to match the other resolvers.

I don't have a horse in the race for either approach; consistency trumps both here and we can always revisit performance if it turns out to be problematic. I was mostly only wondering if we've looked into the possible performance impact of this change already or if we'll do it while/after it goes in.