feat(xds): Add filter state reference-counted resource management#12879
feat(xds): Add filter state reference-counted resource management#12879sauravzg wants to merge 1 commit into
Conversation
- Introduce `SharedResourceManager` to manage reference-counted shared resources across xDS filters and resolvers. - Update `XdsNameResolver` to integrate with `SharedResourceManager` and support filter state retention. - Add `RefCountedRoute` and `RefCountedRouteInterceptor` to ensure resources are reliably retained during call interception and released across all RPC lifecycle termination paths (normal completion, cancellation, and exceptions). - Update `Filter` interfaces and existing implementations (`FaultFilter`, `GcpAuthenticationFilter`) to accommodate filter state sharing.
| * {@link #release(Object)} API designed for xDS filter state cleanup tasks. | ||
| */ | ||
| @Internal | ||
| public final class SharedResourceManager<K, V extends SharedResourceManager.ResourceCloseable> { |
There was a problem hiding this comment.
So, one of the things that I was in thoughts about was , if we should make this class simpler by making this entire map class synchronized (like ReferenceCountingMap in io..xds....security) instead of trying to deal with corner cases around atomic consistency which are difficult to understand and document.
Another thing worth considering is that potentially all operations on the map happen in syncContext since this map is only used when calling buildInterceptor which happens in syncContext , the release also either happens during similar control plane updates so synccontext.
It's possible for the cleanup to happen after an RPC end or close , but that's also covered since we do it inside synccontext using a RouteWrapper.
So, we may entirely not need this class to be thread safe.
So, I want opinions on where we should lean towards. IIUC, the ClusterRefState has semantics where it can be get from any thread and mutations from sync context.
There was a problem hiding this comment.
Moving the synchronization down to the individual resource level and using a lock-free design for the hot path is superior. In a coarse-grained design where we do it at the SharedResourceManager (map) level, every single RPC across different ext-proc addresses would have to acquire a single global lock on SharedResourceManager during acquire and release.
It is not true that all operations on the map happen in syncContext. buildClientInterceptor is called when processing xDS updates on the Control plane in a syncContext. Not the SharedResourceManager.acquire and SharedResourceManager.release. acquire happen on the application's RPC thread and release on the transport thread that calls onClose when the rpc completes.
2 participants
SharedResourceManagerto manage reference-counted shared resources across xDS filters and resolvers.XdsNameResolverto integrate withSharedResourceManagerand support filter state retention.RefCountedRouteandRefCountedRouteInterceptorto ensure resources are reliably retained during call interception and released across all RPC lifecycle termination paths (normal completion, cancellation, and exceptions).Filterinterfaces and existing implementations (FaultFilter,GcpAuthenticationFilter) to accommodate filter state sharing.