Skip to content

Allow per-component configuration in Flux topology#8783

Merged
reiabreu merged 5 commits into
apache:masterfrom
GGraziadei:8710-flux-component-conf
Jun 9, 2026
Merged

Allow per-component configuration in Flux topology#8783
reiabreu merged 5 commits into
apache:masterfrom
GGraziadei:8710-flux-component-conf

Conversation

@GGraziadei

Copy link
Copy Markdown
Contributor

What is the purpose of the change

Add per-component configuration in the flux topology def grammar.

How was the change tested

@GGraziadei GGraziadei changed the title init Allow per-component configuration in Flux topology Jun 2, 2026
@rzo1 rzo1 requested review from reiabreu and rzo1 June 3, 2026 07:25
Comment thread flux/flux-core/src/test/java/org/apache/storm/flux/TCKTest.java
@rzo1

rzo1 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Nice, well-scoped feature that rides on Storm's native component-config mechanism.

  1. Bolt config is applied once per incoming stream, not once per bolt. The block applying memory/CPU/tasks/config runs on every iteration of the streams loop, so a bolt with N inputs calls validateFields() + addConfigurations() N times on the same declarer. It's idempotent, so not a bug, and it matches the existing pattern for setMemoryLoad/setCPULoad/setNumTasks (just redundant=
  2. Validation asymmetry. buildConfig() applies the topology-wide config: with no ConfigValidation, while this PR validates per-component config. Fail-fast is the better behavior, but the asymmetry (an invalid-typed value passes topology-wide yet fails per-component) may surprise users. Consider validating the topology config too in a follow-up, or noting the difference.
  3. README clarity. "Known Storm configuration keys are validated …" is accurate but could read as blanket validation. Suggest adding: "Unknown/custom keys are not validated and are passed through verbatim; validation runs client-side at build time."
  4. Nit: new HashMap<String, Object>() in VertexDef → diamond <> to match the codebase.

@GGraziadei

Copy link
Copy Markdown
Contributor Author

Thank you for the reviews! I have just pushed a new round of commits to address your feedback.

@rzo1
Redundant Loop Execution: I refactored the logic using the new functional switch pattern to ensure the component declarer is defined only once during the first stream, eliminating the redundant calls.
Validation asymmetry: Updated the logic to validate the topology-wide configuration as well, ensuring a consistent fail-fast behaviour across both levels.
Documentation and nit: fixed.

@reiabreu
Regarding the topology-level override test: I did some further investigation. Because the merged configuration is evaluated at runtime, a full verification would technically require an integration test. However, since FluxBuilder acts as a wrapper for TopologyBuilder, and TopologyBuilder's overriding behaviour is already covered under TopologyIntegrationTest#testComponentSpecificConfig, I opted for a lighter approach to avoid test duplication.
I have added a unit test in TCKTest.java to verify that merging the topology and component configurations via Utils.merge() executes without conflicts. If you still feel a full integration test is necessary here, please let me know, and I'll be happy to add it!

@reiabreu

reiabreu commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@rzo1 please check if you are happy to merge it. Cheers

@reiabreu reiabreu merged commit 85a5cbc into apache:master Jun 9, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flux support per-component configuration in BoltDef and SpoutDef

3 participants