Remote registry gRPC client sets no deadline and no keepalive, and neither is configurable: a blackholed connection hangs the caller indefinitely
- Dominant language
- Python
- Stars
- 7.3k
- Forks
- 1.4k
- Avg merge
- 3d 16h
- Merged PRs (30d)
- 15
Description
## Expected Behavior
A registry RPC issued by `RemoteRegistry` should fail within a bounded time when
the connection to the registry server stops making progress, and the bound
should be configurable through `RemoteRegistryConfig`.
## Current Behavior
`RemoteRegistry` builds a channel with no keepalive and issues every RPC with no
deadline, so a connection that stops delivering data but stays `ESTABLISHED`
leaves the caller parked in `epoll_wait` forever.
Three things combine:
1. `RemoteRegistryConfig` (`sdk/python/feast/infra/registry/remote.py`) exposes
`path`, `cert`, `is_tls`, `client_cert`, `client_key` and `authority`. There
is no timeout field and no keepalive field.
2. `RemoteRegistry._create_grpc_channel` populates `options` only with
`grpc.default_authority`. No `grpc.keepalive_time_ms` or related option is set.
3. None of the RPC call sites pass a deadline. `remote.py` has 50
`self.stub.(...)` calls on `v0.64.0` and 51 on `v0.65.0`, with zero
occurrences of `timeout=` in either.
There is also no supported way to substitute an implementation.
`get_registry_config_from_type` validates `registry_type` against the hardcoded
`REGISTRY_CLASS_FOR_TYPE` map with the comment `# We do not support custom
registry's right now`, and `FeatureStore._create_registry` instantiates
`RemoteRegistry` directly, so a subclass is never reachable. This is unlike the
online store, offline store and batch engine, which all accept a dotted class
path.
One related footgun: `RegistryConfig` extends `FeastBaseModel`, whose
`model_config` is `extra="allow"`. An invented key such as `grpc_options` in the
`registry` block therefore validates cleanly and is silently ignored, because
nothing reads it. A user attempting to configure this gets no error.
**Observed impact.** In a long-running batch job that writes to the offline
store, a registry fetch on a blackholed network path never returns. The writer
thread never completes, the write queue fills, every producer blocks on it, and
the pod sits at roughly 1m CPU with 0 restarts until the node is reclaimed.
Nothing in the logs indicates a stuck RPC, because no error is ever raised.
## Steps to reproduce
1. Configure a remote registry:
```yaml
project: demo
registry:
registry_type: remote
path: registry.example:80
```
2. Make the registry endpoint blackhole traffic after the TCP connection is
established — for example drop packets to that address with a firewall rule
(`iptables -A OUTPUT -d -p tcp -j DROP`) after the first
successful call, or route through a proxy that accepts the connection and then
stops responding.
3. Call any registry method, e.g. `store.get_feature_view("some_fv")`.
4. The call does not return. `ss -tnp` shows the socket still `ESTABLISHED`;
`py-spy dump` shows the thread in `epoll_wait` under
`grpc._channel._UnaryUnaryMultiCallable.__call__`. There is no timeout and no
exception.
Adding a timeout through configuration is not possible: `registry.grpc_options`
or `registry.timeout` are accepted by pydantic and ignored, and
`registry_type: my.module.MyRegistry` is rejected by
`FeastRegistryTypeInvalidError`.
### Specifications
- Version: 0.64.0; also verified present on 0.65.0 (latest release) and `master`
- Platform: Linux, Python 3.12, Kubernetes; remote registry over gRPC
- Subsystem: registry — `feast.infra.registry.remote.RemoteRegistry`
## Possible Solution
Two parts, both small:
1. Add optional `timeout` and keepalive fields to `RemoteRegistryConfig`, and
apply them in `_create_grpc_channel`. A default deadline is the load-bearing
half: the hang always has an RPC in flight, so a deadline bounds it, requires
no server agreement, and cannot be refused by an intermediary. Keepalive is
the wider net, since it also catches a channel that rots while idle.
2. Rather than touching 50 call sites, install a
`grpc.UnaryUnaryClientInterceptor` that sets a default `timeout` on any call
issued without one. Because Feast already layers
`GrpcClientAuthHeaderInterceptor` over the channel, the two compose and every
RPC gains a deadline with no call-site changes.
On `grpc.keepalive_permit_without_calls`: it should be left unset, or at least
not defaulted on. gRPC servers enforce
`GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS` (5 minutes by default)
and answer too-frequent pings with a `GOAWAY` carrying `too_many_pings`, which
would kill working connections.
A secondary fix worth considering independently: let `registry_type` accept a
dotted class path, as the online and offline stores already do. That would give
users an escape hatch for this class of problem without a patch.
We currently monkeypatch `RemoteRegistry._create_grpc_channel` to add keepalive
options and wrap the channel in a deadline interceptor. Happy to open a PR along
the lines of (1) and (2) if the approach looks right.
Contributor guide
Research direction
Start in sdk/python/feast/infra/registry/remote.py with RemoteRegistryConfig and RemoteRegistry._create_grpc_channel, then inspect the RPC call sites and existing GrpcClientAuthHeaderInterceptor. Verify how timeout and keepalive settings can be configured without changing every call site. Done means blackholed registry calls fail within a configurable bound and the channel applies the intended keepalive behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- grpc, python
- Domain
- api, backend, networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100