arunkumarucet opened a new pull request, #19434:
URL: https://github.com/apache/pinot/pull/19434
## Summary
A remote (e.g. S3) protobuf descriptor file is re-downloaded on **every
decoder creation** — i.e. on every `OFFLINE→CONSUMING` transition, on every
replica, on every segment rollover. That makes the transition a hard,
non-recoverable dependency on live DNS + object store: a transient DNS blip at
rollover time permanently marks consuming segments `ERROR`, requiring a manual
`POST /segments/{table}/reset`.
This PR keeps fetching the descriptor fresh on every call (so in-place
updates keep propagating exactly as before), but remembers the **last
successfully fetched and parseable content per URI** and serves it **only when
the fetch fails**, so decoder creation survives transient DNS / object-store
outages.
## Production incidents motivating this
Two unrelated production clusters, 4 days apart, identical signature
(details anonymized):
| | Customer 1 | Customer 2 |
|---|---|---|
| DNS failure rate | ~10% of cluster queries, for ~70s | 100%, for ~70s |
| consuming segments ERROR | 5 across 4 tables | 1 (+ replica-loss alert) |
| recovery | manual segment reset (~55 min in ERROR) | next transition once
DNS returned |
Both were triggered by an abrupt K8s node loss removing a CoreDNS pod's
endpoint while it was already unreachable, failing a fraction of all cluster
DNS queries for ~70s.
Failure signature (both):
```
WARN [RealtimeSegmentDataManager_<segment>] Failed to initialize the
StreamMessageDecoder:
java.lang.RuntimeException: Caught exception while creating
StreamMessageDecoder from stream config: ...
Suppressed: software.amazon.awssdk.core.exception.SdkClientException:
Unable to execute HTTP request: s3.us-east-1.amazonaws.com: Temporary
failure in name resolution
ERROR [SegmentOnlineOfflineStateModel] Caught exception while processing
SegmentOnlineOfflineStateModel.onBecomeConsumingFromOffline() for
table: <table>
org.apache.pinot.spi.utils.retry.AttemptsExceededException: Operation failed
after 5 attempts
```
Full call chain (identical in both incidents):
```
SegmentOnlineOfflineStateModel.onBecomeConsumingFromOffline
HelixInstanceDataManager.addConsumingSegment
RealtimeTableDataManager.addConsumingSegment
RealtimeTableDataManager.doAddConsumingSegment
RealtimeTableDataManager.createRealtimeSegmentDataManager
RealtimeSegmentDataManager.<init>
┌─▶ BaseRetryPolicy.attempt ◀── RETRY LAYER 1: 5 attempts
│ RealtimeSegmentDataManager.lambda$new$0
│ RealtimeSegmentDataManager.createMessageDecoder
│ StreamMessageDecoder.init
│ ProtoBufMessageDecoder.init
│ ProtoBufUtils.getDescriptorFileInputStream ◀── this PR adds
the fallback here
│ ProtoBufUtils.getFileCopiedToLocal
│ NoClosePinotFS.copyToLocalFile
│ S3PinotFS.copyToLocalFile
├─────────▶ S3PinotFS.retryWithS3CredentialRefresh ◀── RETRY LAYER 2
│ S3PinotFS.lambda$copyToLocalFile$16
│ DelegatingS3Client.getObject → DefaultS3Client.getObject
└──────────────▶ AWS SDK ◀── RETRY LAYER
3: 4 SDK attempts
ApacheHttpClient.execute
InetAddress.getAllByName → CachedLookup.get ✗ DNS
failure
```
All three existing retry layers share the same fate: they need the outage to
end within the retry budget. Worse, the outer retry budget (~8s across 5
attempts) sits entirely inside the JVM's negative-DNS cache TTL
(`networkaddress.cache.negative.ttl`, default 10s), so after the first real DNS
failure the remaining attempts replay the cached failure without touching the
network — in the measured incident, 5 configured attempts delivered exactly one
real network attempt. A fallback, unlike a retry, does not need the outage to
end.
## The fix
In `ProtoBufUtils.getDescriptorFileInputStream` (used by both
`ProtoBufMessageDecoder` and `ProtoBufRecordReader`):
1. **Fetch fresh on every call** via the existing `PinotFS` copy-to-local
path (unchanged — S3/GCS/ADLS/HDFS all work as before). In-place descriptor
updates propagate at the next rollover, exactly as today.
2. **Validate** the downloaded bytes parse as a descriptor set, so a
corrupt/truncated download can neither be served nor overwrite the remembered
copy.
3. **Remember** the validated content per descriptor URI in a static
in-memory cache (bounded at 64MB total content; descriptors are small).
4. **On fetch failure only**, serve the remembered copy with a WARN. If
nothing was ever fetched successfully (fresh JVM during an outage), rethrow —
behavior unchanged.
Also fixes a temp-dir leak: the download previously left a fresh
`/tmp/pinot-protobuf*` directory behind on every decoder creation.
Behavior summary:
| Scenario | Before | After |
|---|---|---|
| Normal rollover | fetch from remote store | same (plus temp-dir cleanup) |
| Descriptor updated in place | picked up at next rollover | same |
| DNS/S3 outage at rollover, descriptor fetched before on this JVM | segment
→ ERROR, manual reset | ingestion continues on last-known-good copy, WARN
logged |
| Outage, never fetched on this JVM (fresh pod) | fails | fails (unchanged) |
| Corrupt/truncated download | decoder init fails | falls back to last good
copy; corrupt bytes never remembered |
## Scope / limitations
- The fallback covers **descriptor files only**. The jar downloaded by
`ProtoBufCodeGenMessageDecoder` still requires the remote filesystem to be
reachable at decoder-creation time — it needs a file on disk for class loading
and cannot be validated as a descriptor set. Called out in the code docs;
candidate for a follow-up.
- Local (`file://`) descriptor paths are read fresh and never remembered —
no behavior change.
- The remembered copy is served **only** on fetch failure, so a persistent
non-transient failure (revoked credentials, deleted file) keeps ingestion alive
on the old copy until JVM restart, with a WARN per rollover.
## Test plan
- New `ProtoBufUtilsDescriptorCacheTest` (5 tests) using a
counting/fault-injecting `PinotFS`:
- fresh fetch on every call; in-place updates picked up
- fallback to last fetched copy on fetch failure, and recovery after
- fetch failure with no remembered copy propagates
- corrupt download is neither served nor poisons the remembered copy
- local files always read fresh, never remembered
- Full `pinot-protobuf` module suite passes (172 tests), `checkstyle:check`
and `license:check` clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
