Repository navigation
minor: Register Dart enabled historicals via a new DartWorkerService which Dart controller uses for discovering workers - #20508
Merged
Conversation
… of inferring workers solely off service == historical
FrankChen021
reviewed
Oct 8, 2026
FrankChen021
left a comment
Member
There was a problem hiding this comment.
The controller now enrolls workers from the advertised Dart capability and uses the same filtered discovery for message relays. The inventory and discovery views use matching advertised host-and-port identifiers; I found no actionable issues in the changed behavior.
Reviewed 9 of 9 changed files (6 production files and 3 test files), plus the relevant discovery, inventory, service-announcement, and CLI module wiring.
Validation: git diff --check 131989817271bc836e815db738cc2be33fba3538 a970f6c2de7e94ee3140995fe2b154eae7371927 passed. No tests or builds were run; this was a static review.
This is an automated review by Codex GPT-5.6-Luna(max)
gianm
approved these changes
Oct 8, 2026
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.
Description
Instead of coupling the Dart worker to the Historical service, I think it makes sense to tie an announced DartWorkerService component to historicals who enable Dart engine via runtime property. If nothing else it adds some more rich information to the historicals announced capabilities. But it also could provide us future optionality in how Dart workers are registered and used. The recommended guidance in dart docs continues to recommend setting the dart flag in common runtime properties; so early adopters who already do that will see zero impact or behavior change as long as they follow the rolling update order of historicals before brokers in the Druid 39 upgrade.
Mostly things will stay exactly the same, but now the Dart controller will only enroll actual Dart workers instead of all historicals without first checking if they run Dart in the first place. This would avoid the controller from sending Dart queries to a historical who explicitly disabled Dart.
Release note
Dart workers on historicals are announced and discovered by the broker with a new DartWorkerService mechanism. Operators whose clusters have enabled dart should be sure to follow the standard rolling upgrade/downgrade order where historicals restart before broker during upgrade and the reverse during downgrade.
Key changed/added classes in this PR
DartControllerContextDartMessageRelaysDartWorkerServiceThis PR has: