New cache interface proposal #718
Conversation
…e uniform between sotw and delta Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
…lso compatible with sotw Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
| // The versions are: | ||
| // - delta protocol: version of the specific resource set in the response | ||
| // - sotw protocol: version of the global response when the resource was last ACKed | ||
| GetKnownResources() map[string]string |
There was a problem hiding this comment.
The goal is to remove the current global version behavior in the future. It would allow to handle both protocols in a more uniform manner, as well as supporting partial sotw responses for non-wildcard requests
|
|
||
| type FetchRequest interface { | ||
| Request | ||
| GetResourceNames() []string |
There was a problem hiding this comment.
Fetch has no subscription notion
| // Get the context provided during response creation. | ||
| GetContext() context.Context | ||
|
|
||
| // Get the Constructed DiscoveryResponse |
There was a problem hiding this comment.
Imo those should not be in Cache, as those depend on the server and the cache does not really need to know what requests/responses are passed
For now I kept them there to limit the impact
| ) | ||
|
|
||
| type resourceWithNameAndVersion interface { |
There was a problem hiding this comment.
The idea is to avoid constraints on the resources that are actually not needed. Any added resource version in the cache is already set with a name
For the version it avoids having to rehash the resource. In the future we could also avoid the double serialization we currently do, which is quite expensive in CPU
| var _ DeltaResponse = &rawDeltaResponse{} | ||
|
|
||
| // RawDeltaResponse is a pre-serialized xDS response that utilizes the delta discovery request/response objects. | ||
| type rawDeltaResponse struct { |
There was a problem hiding this comment.
Made private. This allows us to evolve Cache implementations with less impact on users
| // * detect the version change, and return the resource (as an update) | ||
| // * detect the resource deletion, and set it as removed in the response | ||
| streamState.GetResourceVersions()[resource] = "" | ||
| streamState.GetKnownResources()[resource] = "unsubscribed" |
There was a problem hiding this comment.
This is for sure hacky (though not much more than before), while allowing the use of "" for the standard case of "non-existent" resource
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
This draft PR is proposing a new interface for the Cache package
The main goals are:
This PR is not yet finalized (e.g. it doesn't compile currently), but it shows a first proposal. It brings a lot of change and the final implementation would likely be broken in pieces