This repository has been archived by the owner on Nov 1, 2022. It is now read-only.
Cache the generators of patchUpdated configurations #2805
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2801
This change assumes that the generator commands of
patchUpdated
configurationshave a constant output throughout the execution of an synchronization/policy
update/image release operation. This was already assumed, to certain extent, by
patchUpdated
's implementation since, for each workload/container update we runthe
generators
in order to refresh thepatchFile
.In practice, this change caches the result of the generators throughout the
lifespan of the
ConfigFile
variable (which, currently, is the execution of asynchronization, policy update or release operation). However, I'm not entirely
happy about this, since the lifespan of
ConfigFile
may change in the future. Iwould rather explicitly track what the cache depends on but:
Using cache keys is probably not enough (Maybe use the Git Hash of the
filesystem? But what if there are file modifications? and what about external
network dependencies?).
ConfigFile
is decoupled from the git repository implementation I would likeit remain this way. So I would rather not invoke git commands in the
ConfigFile
implementation.Alternatively to cache keys we could bookkeep the cache outside of
ConfigFile
but that's not easy since:by the generators (which in
patchUpdated
isn't the same) whichare not accessible external.
Config
file is a few layers under themanifests.Store
Maybe I will find a better solution later on.
Anyways, this cache should be effective because update/release operations are
subdivided into individual workload/container operations. This smaller
operations can be numerous, and each of them requires obtaining the result of
the
generators
execution to recompute thepatchFile
. On top of that, everyimage release operation has a verification stage, which also requires the
generators
result.It's worth noting that we cannot do the same sort of caching for
commandUpdated
configurations, since theupdater
invocations are supposed tocause side-effects in the
generators
' result. If it's of any consolation,commandUpdated
configurations don't invoke thegenerators
for updates,but, we do call the
updaters
which may be doing that under the hood. Theconclusion is that
commandUpdated
configurations should do the cachingthemselves.