[cache/linear] Expose cache version#1467
Conversation
Signed-off-by: Kateryna Nezdolii <kateryna.nezdolii@isovalent.com>
valerian-roche
left a comment
There was a problem hiding this comment.
Can you provide a rationale to increase the public interface?
I am hoping to fully remove this "version" from the linear cache as it's irrelevant in delta mode and is arbitrary in sotw.
| } | ||
|
|
||
| func (cache *LinearCache) GetVersion() uint64 { | ||
| return cache.version |
There was a problem hiding this comment.
This cannot be returned without concurrency protection. version is only touched under lock today.
Also the version integer is an internal detail. The version from a public API point-of-view is a string with the provided version prefix, not this integer
|
The need for this change occurred while migrating cilium custom xds control plane to use go control plane in ADS SOTW mode. In cilium control plane we send core Envoy resource and custom resources (like network policy). Since Snapshot cache (which exposes xds resource version) does not support custom resource types (we try to avoid to use TypedExtensionConfig for now), I have to use linear cache for custom resource types (and then combine linear caches with snapshot cache into a mux cache). Cilium control plane registers "await" objects (based on xds node Id, resource version set by cache and type url) and performs certain actions based on when await objects have been acked or if they timed out. In our case we care about resource version which is being set by cache and being sent on the stream it does not need to be user friendly or human readable. Since snapshot cache exposes resource version it would be meaningful to also expose resource version in linear cache to be able to use those cache types in mux cache. String resource version would work as well, we will just parse it on out side to extract the integer value (we care about what is being actually sent on stream). |
Description
Expose version in linear cache as public getter.
#
Type of change
Please delete options that are not relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: