Jackie-Jiang opened a new pull request, #19583:
URL: https://github.com/apache/pinot/pull/19583

   ## Summary
   
   Follow-up to #19530 (and the earlier attempt in #17605).
   
   #19530 interns the instance id used as the key of the enabled / routable 
server maps so it shares identity with the instance ids Jackson interns when 
decoding the `IdealState` / `ExternalView` map fields. `ServerInstance` itself 
still read the id from `InstanceConfig.getInstanceName()`, which is the 
ZNRecord id: a JSON value that Jackson does not intern. Since `ServerInstance` 
is rebuilt on every instance config refresh, each refresh produced a fresh copy 
of every server's id, and long-lived holders of `getInstanceId()` (routing 
stats, failure detector, MSE `QueryServerInstance`, per-server maps) each 
retained the copy from whichever refresh they observed, without the `==` fast 
path on lookups.
   
   This PR moves the interning into `ServerInstance`:
   - `ServerInstance` interns `_instanceId` in both constructors and documents 
the contract on the constructor and `getInstanceId()`.
   - `BaseBrokerRoutingManager` builds the `ServerInstance` first and keys 
`enabledServers` / `_enabledServerInstanceMap` by 
`serverInstance.getInstanceId()`, dropping its own `intern()` call so interning 
happens in one place.
   - `ServerInstanceTest` gains `testInstanceIdInterned`; the #19530 regression 
test in `BrokerRoutingManagerTest` now asserts `assertSame` between the 
server's id and the Jackson-decoded IS/EV key instead of `assertEquals`.
   
   ### Behavior note
   
   Because the key is now taken from the constructed instance, 
`enabledServers.add` happens after the `ServerInstance` constructor. If the 
constructor throws on a malformed config (unparseable hostname, port or pool; 
the other fields are parsed leniently by Helix), a previously enabled server is 
now treated as disabled and dropped from routing until its config is fixed, 
instead of staying routable with the stale `ServerInstance`. This matches the 
"ignoring it" intent of the surrounding catch block.
   
   🤖 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]

Reply via email to