Add streaming early-exit JsonPath extractor for simple linear paths#18972
Open
xiangfu0 wants to merge 3 commits into
Open
Add streaming early-exit JsonPath extractor for simple linear paths#18972xiangfu0 wants to merge 3 commits into
xiangfu0 wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #18972 +/- ##
============================================
+ Coverage 65.21% 65.24% +0.02%
Complexity 1405 1405
============================================
Files 3418 3421 +3
Lines 214642 214887 +245
Branches 33919 33980 +61
============================================
+ Hits 139981 140205 +224
- Misses 63366 63386 +20
- Partials 11295 11296 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
xiangfu0
force-pushed
the
claude/streaming-json-path-extractor-19a1e2
branch
from
ad8ac93 to
a7147f4
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Jackson streaming-based JsonPath extractor optimized for simple linear paths and wires it into both query-time scalar functions and ingestion-time jsonExtractScalar, behind two default-OFF feature flags to preserve existing Jayway semantics.
Changes:
- Introduces
SimpleJsonPath+StreamingJsonPathExtractorand a global mode switch (JsonPathFastPathMode) for fast-path enablement / early-exit behavior. - Wires the fast path into
JsonFunctions.jsonPath*andJsonExtractScalarTransformFunction, with startup-time config seeding viaServiceStartableUtils. - Adds differential + fuzz testing for parity against the Jayway fallback, plus fast-path re-runs of existing JSON function / transform-function test suites, and a JMH benchmark.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java | Adds cluster config keys for fast-path enablement and early-exit. |
| pinot-common/src/main/java/org/apache/pinot/common/function/JsonPathFastPathMode.java | Adds global volatile switches (system-property seeded) for fast-path and early-exit. |
| pinot-common/src/main/java/org/apache/pinot/common/function/SimpleJsonPath.java | Compiles/caches the restricted “simple linear” JsonPath subset for streaming resolution. |
| pinot-common/src/main/java/org/apache/pinot/common/function/StreamingJsonPathExtractor.java | Implements single-/multi-path streaming extraction via Jackson JsonParser. |
| pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java | Routes eligible jsonPath* calls through the streaming extractor when enabled. |
| pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStartableUtils.java | Seeds fast-path flags from cluster config during service startup. |
| pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunction.java | Applies streaming fast path to ingestion jsonExtractScalar for STRING/BYTES inputs when eligible. |
| pinot-common/src/test/java/org/apache/pinot/common/function/StreamingJsonPathExtractorTest.java | Differential + fuzz parity tests (Jayway oracle), plus explicit early-exit divergence assertions. |
| pinot-common/src/test/java/org/apache/pinot/common/function/JsonFunctionsFastPathTest.java | Re-runs existing JsonFunctionsTest under fast-path enabled. |
| pinot-core/src/test/java/org/apache/pinot/core/operator/transform/function/JsonExtractScalarTransformFunctionFastPathTest.java | Re-runs existing JsonExtractScalarTransformFunctionTest under fast-path enabled. |
| pinot-common/src/test/java/org/apache/pinot/common/function/JsonPathFastPathModeTest.java | Tests startup wiring/config assignment for the two feature flags. |
| pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkJsonPathExtraction.java | Adds JMH benchmarks comparing Jayway vs streaming extraction (single + multi-path). |
xiangfu0
force-pushed
the
claude/streaming-json-path-extractor-19a1e2
branch
2 times, most recently
from
95eca38 to
0a618e0
Compare
Ingestion and query-time JsonPath extraction goes through Jayway
(parse(json).read(path)), which builds a full Jackson DOM of the document
and only then walks to the field. This happens once per row per derived
column and is a major ingestion CPU cost.
This adds a streaming Jackson-based extractor for simple linear paths
($ followed only by .key, ['key'], [int]), falling back to Jayway for
anything else (wildcards, deep scan, filters, unions, slices, negative
indices, non-String/byte[] input, and any document whose first
non-whitespace char is not { or [).
Feature-flagged, both flags default OFF:
- pinot.jsonpath.fastpath.enabled - gate the fast path
- pinot.jsonpath.fastpath.earlyExit.enabled - stop at the leaf
The default (full-scan) path is byte-for-byte identical to Jayway,
including duplicate-key last-wins and raising on a document malformed
inside its root value. Early exit trades those two behaviors (first
duplicate wins; a malformed tail after the leaf is not seen) for a large
speedup when the field appears early; both inputs are RFC-8259-undefined
or corrupt.
Server-local compute only: no wire format, segment format, or config
schema change, so mixed-version safe with no migration.
Correctness gate: StreamingJsonPathExtractorTest is a differential test
with the production Jayway fallback as oracle - the same jsonPath*
entry points run flag-off vs flag-on and must agree, over a hand-picked
matrix plus 60k fuzz iterations, the byte[] overloads, and the
BigDecimal context. JsonFunctionsFastPathTest and
JsonExtractScalarTransformFunctionFastPathTest re-run the existing
corpora through the fast path.
JMH (BenchmarkJsonPathExtraction, Apple M, 688B nested payload):
jsonPathString 458k -> 836k ops/s (1.8x, the Jackson lex floor) at exact
parity; 5.09M (11x) with early exit when the field is early. Four
columns from one document via the single-pass multi-path extractor:
106k -> 763k ops/s (7.2x) at exact parity.
SimpleJsonPath.compile returns null for non-linear paths, so the field is null whenever the extraction falls back to Jayway. Mark it @nullable and reword the Javadoc to state the null case, per review feedback.
xiangfu0
force-pushed
the
claude/streaming-json-path-extractor-19a1e2
branch
from
0a618e0 to
10c200e
Compare
Review found three real issues: 1. Enabling the fast path REGRESSED non-simple paths. JsonFunctions.jsonPath compiles a SimpleJsonPath per row, and the Guava maximumSize cache takes a segment lock on every read to maintain its LRU recency queue. That is a second contended cache read per row on top of Jayway's own, measured at ~+11% on a rejected path at 8 threads (invisible single-threaded). Replaced with a ConcurrentHashMap: lock-free reads, growth bounded by refusing new entries when full (re-parsing a path is a few dozen char comparisons, so the miss is cheaper than maintaining an eviction policy). 2. The "byte-for-byte identical to Jayway" claim was too strong. Jayway builds a DOM of the whole document, so it materializes and validates values the path never addresses; this extractor skips those subtrees. A value Jackson can lex but not materialize therefore raises in Jayway and not here - an over-long string (> maxStringLength), and, under useBigDecimal, a float whose exponent overflows an int. Both only differ when the value sits OUTSIDE the addressed path; when it IS the addressed value, both raise. This is inherent to the optimization (matching it would mean materializing every string in every document) and the extractor never returns a different value for the addressed path - it only declines to fail on values the caller did not ask about. Documented on the class and pinned with a test so it cannot widen unnoticed. 3. The useBigDecimal context - which backs jsonExtractScalar STRING/BIG_DECIMAL, the most common result types - was never fuzzed, because the existing fuzzers all route through JsonFunctions, which hard-codes it off. That gap is what hid (2). Added a dedicated 20k-iteration BigDecimal fuzz against its own oracle. Also addressed review comments: guard a null jsonPath so it falls back to Jayway rather than raising a different exception, validate the multi-path out[] length, correct the startup warning that claimed to ignore a flag it actually applies, and reset both flags (not just one) in the fast-path test classes, which mutate global state shared across the fork.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ingestion and query-time JsonPath extraction goes through Jayway
(
parse(json).read(path)), which builds a full Jackson DOM of the document and only then walks tothe field — once per row per derived column, a major ingestion CPU cost. This adds a streaming
Jackson-based extractor for simple linear paths (
$followed only by.key,['key'],[int]), used by both re-parse call sites:JsonFunctions.jsonPath(andjsonPathString/Long/Double/Array/Exists, which route through it)JsonExtractScalarTransformFunction(ingestiontransformConfigs)Everything else falls back to Jayway: wildcards, deep scan (
..), filters, unions, slices,negative indices, non-
String/byte[]input, and any document whose first non-whitespace characteris not
{or[(bare scalars,null, empty, whitespace, BOM, plain text).Feature flags — both default OFF
pinot.jsonpath.fastpath.enabledpinot.jsonpath.fastpath.earlyExit.enabledPure kill-switch back to Jayway. Server-local compute only — no wire format, segment format, or
config-schema change, so mixed-version safe with no migration.
Semantics
The default (full-scan) path returns the same value as Jayway as Pinot configures it
(
JacksonJsonProvider+JacksonMappingProvider+SUPPRESS_EXCEPTIONS) for every input, includingduplicate-key last-wins and raising
InvalidJsonExceptionon a document malformed anywhere inside itsroot value. Content after the root value is ignored, exactly as
ObjectMapper.readValueignores it.One deliberate difference. Jayway builds a DOM of the whole document, so it materializes and
validates values the path never addresses. This extractor skips those subtrees, so a value Jackson can
lex but not materialize makes Jayway reject the document while this returns the requested value.
Exactly two such values exist, and only when they sit outside the addressed path:
StreamReadConstraints.maxStringLength(20,000,000 chars);useBigDecimal(which backsjsonExtractScalar(..., 'STRING')/'BIG_DECIMAL'), a floatliteral whose exponent overflows an
int, e.g.1e999999999999.When the offending value is the addressed one, both raise, so the results still agree. This is
inherent to the optimization — matching Jayway here would mean materializing every string in every
document, which is precisely the cost this change exists to avoid. The extractor never returns a
different value for the addressed path; it only declines to fail on values the caller did not ask
about, and it is the safer of the two (it never allocates the 20 MB string or the pathological
BigDecimal). Pinned by a test so it cannot widen unnoticed.Early exit trades two behaviors for speed when the field appears early, since the tail is never
read: (1) duplicate keys resolve to the first occurrence; (2) a document malformed strictly after
the addressed field no longer raises. Both inputs are RFC-8259-undefined or corrupt, and the switch
is off by default.
Correctness gate
StreamingJsonPathExtractorTestis a differential test with the production Jayway fallback as theoracle: the same
jsonPath*entry points run flag-off vs flag-on and must agree, over ahand-picked matrix (missing paths, invalid JSON, null/empty/whitespace/BOM, type mismatches, nested
arrays, arrays of objects, deep nesting, unicode/surrogates, escaped/dotted/duplicate keys,
big/precise numbers) plus 60k randomized fuzz iterations, the
byte[]overloads (incl. multibyteand invalid-UTF-8), and the
USE_BIG_DECIMAL_FOR_FLOATScontext. Early-exit divergences areasserted explicitly.
JsonFunctionsFastPathTestandJsonExtractScalarTransformFunctionFastPathTestre-run the existing corpora through the fast path.
Performance
JMH (
BenchmarkJsonPathExtraction, Apple M, ~688-byte nested payload, 4×5s warmup / 6×5s measure);the
fastPathEnabled=falsearm is the current Jayway baseline, measured in the same run:jsonPathString, one column1.8x is the Jackson lexing floor: exact parity requires reading the whole root value, so a bare
nextToken()-to-EOF loop (~907 ns) is already close to the full extract (~919 ns). The larger winscome from early exit (undefined-input tradeoff) and from single-pass multi-path extraction, which
must walk the document anyway and so gets exact parity for free — the right target for
transformConfigspulling N columns from one source.On representative production logs
To validate on non-synthetic data, both the correctness and the throughput were re-measured on a
corpus of 50k Vector log lines (~1.4KB nested JSON each), in two variants:
messageas astringified-JSON blob, and
messageas a nested object (2% of object-variant rows carry a plainnon-JSON string message).
messageis the last top-level field, so$.message.*is a worst-caselate extraction. The transform extracts a realistic set of 12 columns (6 top-level dimensions plus
6 fields under
message).Correctness: 100k lines × 15 paths = 1.5M extractions compared against Jayway, 0 differences
(full-scan mode). Real production JSON — duplicate-free, deeply nested, mixed string/object
message, unicode — is a stronger differential corpus than the randomized fuzzer.Throughput (JMH, ops/s; string / object variant):
$.message.status), full scanjsonPathStringend-to-end (fast path off → on)Single-column is a modest ~1.4–1.6x here (vs 1.8x on the smaller synthetic payload): these docs are
larger and the target fields sit at the end, so full-scan parity reads almost the whole document —
the lex floor. The multi-column single-pass number is 13–15x, larger than the synthetic 7.2x,
because Jayway re-parses the full ~1.4KB DOM once per column (
jaywayMultiColumn≈ single-field ÷12) — exactly the per-row-per-column re-parse this change removes. This is the case for wiring the
single-pass extractor into
jsonExtractScalar(follow-up).(The production-log harness reads local NDJSON files and so is not committed; it is reproducible
against any NDJSON via
-Dbench.dir. The committedBenchmarkJsonPathExtractioncovers thesynthetic payload above.)
Testing
pinot-common: 575 tests green (incl. the differential + fuzz).pinot-core: 218 green(
JsonExtractScalarTransformFunctionTest+ its fast-path subclass,JsonExtractScalarTest,JsonExtractIndexTransformFunctionTest). spotless / checkstyle / license clean. Additionallyvalidated against 100k real production log lines (1.5M extractions) with zero differences from
Jayway, as noted under Performance.
Notes for reviewers
earlyExitchanges results, so it must be flipped cluster-wide, not mid-rolling-restart — ahalf-restarted cluster would have servers disagree within one scatter-gather.
enabledalone isresult-preserving and rolls normally.
extractis included and benchmarked but not yet wired intojsonExtractScalar(which would need sibling transform functions to share one pass over the sourcecolumn); that is the follow-up where the 7x lands in ingestion.