xiangfu0 opened a new pull request, #19530:
URL: https://github.com/apache/pinot/pull/19530
Broker routing looks up an enabled `ServerInstance` for every selected
segment. Canonicalize instance-config IDs when adding enabled servers so these
lookups can use reference equality with the IDs decoded from assignment-map
keys. The added work is one `intern()` call per enabled server during config
refresh.
Related to #17605. This change is limited to instance-config IDs; it does
not walk or rebuild IdealState/ExternalView assignment maps, add per-query
interning, or change the `ServerInstance` API.
The regression test deserializes instance configs, checks canonical
enabled-map keys, and verifies required/optional segment routing and updated
gRPC ports across two config refreshes. An identity check with current Helix
2.0.1 / Jackson 2.22.2 also confirmed that the assignment-map IDs were already
interned while the decoded config ID was a separate equal string.
### Benchmark
`BenchmarkRoutingInstanceIdLookup` compares separately decoded equal
config-ID keys with canonical keys in the same warmed `ConcurrentHashMap`
lookup loop. Each invocation performs the indicated number of selected-segment
lookups across 100 servers and returns a checksum.
| Lookups per invocation | Baseline (µs/invocation) | Intern config IDs
(µs/invocation) |
|---:|---:|---:|
| 100 | 0.398–0.414 | 0.1595–0.1604 |
| 10,000 | 39.50–40.42 | 15.37–15.45 |
Ranges are the means of forward/reverse variant-order passes, not confidence
intervals. Apple M4 Pro, macOS arm64, OpenJDK 25.0.4, G1, `-Xms1g -Xmx2g`, JMH
1.37; one worker, two forks per pass, three 1-second warmups and four 1-second
measurements, GC profiler. Both orders therefore provide four forks per case.
The machine was shared; this measures isolated lookup cost, not full
routing/query latency, production QPS, or p99. It does not establish a broker
retained-heap reduction.
<details>
<summary>Per-fork means (µs/invocation)</summary>
| Lookups | Variant | Forward fork 1 | Forward fork 2 | Reverse fork 1 |
Reverse fork 2 |
|---:|---|---:|---:|---:|---:|
| 100 | baseline | 0.3975 | 0.3985 | 0.4119 | 0.4152 |
| 100 | intern config ID | 0.1587 | 0.1621 | 0.1589 | 0.1602 |
| 10000 | baseline | 39.1719 | 39.8356 | 40.3013 | 40.5441 |
| 10000 | intern config ID | 15.3248 | 15.4104 | 15.4566 | 15.4414 |
</details>
Reproduce with JDK 25 after building `pinot-perf`:
```sh
./mvnw -pl pinot-perf -am package -DskipTests
java -cp 'pinot-perf/target/pinot-perf-pkg/lib/*' org.openjdk.jmh.Main \
'org.apache.pinot.perf.BenchmarkRoutingInstanceIdLookup.lookupSelectedSegments'
\
-wi 3 -i 4 -w 1s -r 1s -f 2 -t 1 -prof gc \
-p _selectedSegments=100,10000 -p _servers=100 \
-p _variant=baseline,instance_config_intern \
-jvmArgs '-Xms1g -Xmx2g -XX:+UseG1GC' \
-rf json -rff routing-instance-id-lookup.json
```
Repeat with `-p _variant=instance_config_intern,baseline` for the
reverse-order pass.
### Validation
- 65 routing tests passed: `BrokerRoutingManagerTest`,
`BrokerRoutingManagerConcurrencyTest`, `RemoteClusterBrokerRoutingManagerTest`,
and `MultiClusterRoutingManagerTest`.
- `pinot-broker,pinot-perf` and their dependencies compiled through the
Maven test lifecycle on JDK 25.
- `spotless:apply`, `license:format`, `checkstyle:check`, and
`license:check` passed on both affected modules.
- Targeted `javac -Xlint:all,-dangling-doc-comments` compilation passed with
no warnings on added lines; both compiled benchmark variants passed a smoke run.
The targeted lint classpath includes JetBrains annotations because the
existing `zstd-jni` dependency declares them provided; enabling deprecation
diagnostics without that jar fails in unchanged Zstandard code. The excluded
dangling-doc-comments warning comes from the repository-mandated Javadoc-style
ASF license headers. No dependency or compiler configuration is changed by this
PR.
--
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]
