suvodeep-pyne opened a new pull request, #20067:
URL: https://github.com/apache/pinot/pull/20067
Part 1 of 3. This PR adds the classifier only and changes no runtime
behaviour. Part 2 will add the server and controller call sites and the gauges;
part 3 will add Kinesis and Pulsar fixes.
## Problem
When a realtime table's stream refuses Pinot's request, the failure looks
like any other stall: a revoked SASL credential, a removed topic ACL, a Kinesis
`AccessDeniedException`, a schema registry answering 403. Pinot has no notion
of the source's verdict. Every site that sees the typed exception discards it,
so operators find out late and have to read server logs.
## Approach
Classify a failure by what the source said about our request:
- `AUTH`: the source refused us. This covers a rejected credential, a denied
permission or access policy, and a deliberate "does not exist or not
authorized".
- `SOURCE_READ`: the source accepted us but couldn't serve the data, or
never answered (DNS, connect, TLS, timeout).
- Anything else is not classified. That includes Pinot's own clock, its
ambient identity failing to load, local config, content that won't parse, and
cancellation.
`AUTH` carries a bounded sub-reason (`authentication`, `authorization`,
`missing_or_denied`, `unspecified`), but only for log lines. On streams the
line between authentication and authorization is often invisible; for example,
MSK IAM reports a policy denial as a SASL authentication failure.
```mermaid
flowchart LR
F[Failure from a stream, registry or catalog client] --> W[Split into
chains: causes, then each suppressed exception]
W --> S{Deepest source rule<br/>that decides}
S -->|built-in rules| V[Verdict]
S -->|registered rules,<br/>SDKs Pinot doesn't ship| V
S -->|none decided| G[Generic java.net / TLS rules] --> V
E[Evidence carrier:<br/>whose credential, dropped codes] -.-> S
V --> A{Across chains:<br/>AUTH beats SOURCE_READ}
A --> C[Classification:<br/>category, sub-reason, code, target]
```
## Key changes (`pinot-spi`, `org.apache.pinot.spi.ingestion.error`)
- `IngestionSourceErrorClassifier.classify(Throwable)` is pure and never
throws.
- Within a chain, the deepest source rule that decides wins. A rule that
says "this is ours, not the source's" (our clock, an ambient credential that
failed to load) therefore can't be overridden by a wrapper above it.
- Generic JDK network rules apply only when no source rule decided.
- Built-in rules cover kafka-clients 3.x and 4.x, the Confluent
schema-registry client, the AWS SDK v2 (Kinesis, STS, KMS and Glue codes), the
Pulsar client and admin APIs, gRPC/gax, and Kerberos and OAuth logins.
- Classes are matched by name anywhere in the hierarchy, because SDKs load
in plugin classloaders and two kafka-clients versions can coexist. Codes are
read reflectively and fail closed.
- A message is read only where the client stringified its evidence. Each
such rule has a fixture.
- `IngestionSourceErrorEvidence` is an SDK-free carrier attached as a
suppressed exception, so the thrown type doesn't change. It records whose
credential a client used, which decides whether an AWS or gRPC credential
rejection is the table owner's or Pinot's own. It also records codes a client
is about to drop. It holds only bounded values and never message text.
- `IngestionSourceErrorClassifier.registerRule` lets a downstream plugin add
rules for an SDK Pinot doesn't ship. A registered rule decides one throwable at
a time and only where no built-in rule did. Registration order never changes an
outcome.
- `IngestionSourceErrorClassifier.registerGenericRule` adds a rule that
reads the transport rather than a source's answer, such as a file missing under
a path the caller knows is remote. Like the built-in network rules, it applies
to a chain only when no source rule decided it, and after those built-in rules
on each exception.
- A carrier can mark a source boundary (`setBoundary`). When any exception
in a failure carries one, only the parts at or beneath a boundary are read.
This is for callers whose one operation also touches storage that isn't the
source, such as a batch job's deep-store reads. It's off by default; stream
sites don't use it. The classification also exposes the deciding exception and
its evidence, so such a caller can name the resource in its own log line.
## Where to start
`IngestionSourceErrorClassifier` (the walk and precedence), then
`IngestionSourceErrorRules` (one section per SDK), then
`IngestionSourceErrorClassifierTest` for the contract.
## Testing
Fixture tests run in each plugin module against the SDK that module ships,
real client paths included where cheap:
| Suite | What it covers |
|---|---|
| `pinot-spi` | The contract: precedence, registration-order independence,
generic rules, a cause shared by two branches read under each branch's
evidence, source boundaries, evidence on a throwable with suppression disabled,
bounded evidence, classification never throwing on a hostile exception or rule,
deep suppressed nesting on a small stack. |
| `pinot-kafka-base` (3.9.2), `pinot-kafka-4.0` (4.2.0) | Broker
SASL/ACL/TLS rows. A real `KafkaConsumer` against a local OAuth token endpoint
stub (401, 403, 400 `invalid_grant`/`unauthorized_client`/`invalid_request`,
503, refused). An unresolvable bootstrap and an unreadable keystore through the
real client. An isolated-classloader case. |
| `pinot-confluent-avro` | Pinot's real decoder against a local registry
stub: 401, 403, an HTML 403 from a proxy, 404, 429, 503, a timeout,
unreachable, bad bytes. |
| `pinot-kinesis` | AWS codes, credential-mode cases, STS role and
web-identity cases, clock skew, SDK retry records, credential-provider
failures, and a real `KinesisClient` against an endpoint that drops every
request. |
| `pinot-pulsar` | Client and admin rows, including the OAuth error buried
under the 5.0 bridge. |
| `pinot-gcs` | gRPC/gax status rows and a real google-auth token refresh
(revoked key, clock skew, unreachable token endpoint). |
### Verification
An independent verifier ran each check at this head and at the base. At the
base, every test class is red because the classifier doesn't exist.
| Check | Head | Verdict |
|---|---|---|
| Fixture rows and traps for the SDKs Pinot ships | All pass: 23 + 14 + 14 +
8 + 7 + 11 + 9 tests; probes through the real SDKs agree | pass |
| Registered generic rules, over a corpus of 296k exception trees | Never
override a source decision; respect boundaries; order-independent; a throwing
rule counts as no decision; with none registered, nothing changes. The contract
test fails on each of six one-line breaks of this. | pass |
| An SDK class loaded by a separate classloader, including two Kafka
versions side by side | Classifies correctly | pass |
| The evidence carrier survives a throwable with suppression disabled, and
holds no secrets | Holds | pass |
| A classifier or reflection failure never escapes `classify()` | Holds
against hostile exceptions, rules, cycles and shared tails | pass |
Known gaps:
- A schema registry rejecting our client certificate under TLS 1.3 surfaces
with no TLS class, so it isn't classified.
- A schema registry's OAUTHBEARER identity-provider rejection isn't
reachable with the jars Pinot ships.
`pinot-kafka-4.0` gains a test-only jose4j dependency, because kafka-clients
4.x needs it to build an OAUTHBEARER client and doesn't declare it.
--
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]
