PartitionManager can disable an exporter #18512
Merged
Conversation
This file contains 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
deepthidevaki
commented
May 15, 2024
...munda/zeebe/broker/system/partitions/impl/steps/ExporterDirectorPartitionTransitionStep.java
Show resolved
Hide resolved
npepinpe
approved these changes
May 23, 2024
There was a problem hiding this comment.
🚀
However, can we sync f2f on the overall expected UX for managing exporters once this is merged? I like how we're breaking things down at the moment into small updates, but I feel like I may sometimes be missing the forest for the trees, and right now I have some confusion on the expected final UX in terms of:
- Initial exporter configuration
- Modifying exporter via the static configuration
- Adding new exporters
- Removing exporters
- Removing exporters from the dynamic config
...r/src/main/java/io/camunda/zeebe/broker/system/partitions/PartitionConfigurationManager.java
Outdated
Show resolved
Hide resolved
...munda/zeebe/broker/system/partitions/impl/steps/ExporterDirectorPartitionTransitionStep.java
Outdated
Show resolved
Hide resolved
...munda/zeebe/broker/system/partitions/impl/steps/ExporterDirectorPartitionTransitionStep.java
Show resolved
Hide resolved
On start up, PartitionManager gets the configuration from the configuraiton manager. This can be used to start partitions with the correct configuration instead of using the static configuration.
deepthidevaki
force-pushed
the
dd-18419-partition-disable-exporter
branch
from
43187d3
to
fdd4ef5
Compare
github-actions
bot
added
component/operate
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 23, 2024
## Description To simplify the changes required for this PR, we removed the experimental flag for dynamic configuration. This helps to remove the extra code needed to support only static configuration. The main changes in this PR are: 1. The current dynamic partition config is tracked in `PartitionContext`. The initial value of it is received when the partition is bootstrapped. We reuse the `DynamicPartitionConfig` defined in `ClusterConfiguration`. Alternately, we can define a new object internal to `partitions`. But at the moment, it will look identical to the other one. So to keep it simple, we reused the existing class. When we add new configuration parameters to it, we can define a new one if needed. 2. When a config is changed, a call back to the `PartitionManager` will be invoked. The implementation of this call back reacts to the configuration change and also updates the config in the `PartitionContext`. In this case, the call back is `PartitionManager::disableExporter` which disables the exporter in `ExporterDirector`. 3. `ExporterDirectorTransitionStep` reads the latest config and uses that to initialize the exporters during role transitions. Only enabled exporters are given to the `ExporterDirector`. Thus already disabled exporters are not re-enabled. ## Related issues closes #18419
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
To simplify the changes required for this PR, we removed the experimental flag for dynamic configuration. This helps to remove the extra code needed to support only static configuration.
The main changes in this PR are:
PartitionContext. The initial value of it is received when the partition is bootstrapped. We reuse theDynamicPartitionConfigdefined inClusterConfiguration. Alternately, we can define a new object internal topartitions. But at the moment, it will look identical to the other one. So to keep it simple, we reused the existing class. When we add new configuration parameters to it, we can define a new one if needed.PartitionManagerwill be invoked. The implementation of this call back reacts to the configuration change and also updates the config in thePartitionContext. In this case, the call back isPartitionManager::disableExporterwhich disables the exporter inExporterDirector.ExporterDirectorTransitionStepreads the latest config and uses that to initialize the exporters during role transitions. Only enabled exporters are given to theExporterDirector. Thus already disabled exporters are not re-enabled.Related issues
closes #18419