[chore](cloud) Support dynamic recycler instance filter config#63822
Open
wyxxxcat wants to merge 1 commit into
Open
[chore](cloud) Support dynamic recycler instance filter config#63822wyxxxcat wants to merge 1 commit into
wyxxxcat wants to merge 1 commit into
Conversation
Collaborator
Author
|
run buildall |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Collaborator
Author
|
/review |
Contributor
There was a problem hiding this comment.
The PR is small and focused, but it does not currently achieve its stated goal of allowing runtime config updates to affect recycler instance filtering.
Critical checkpoint conclusions:
- Goal/test: The goal is to make recycler whitelist/blacklist changes effective without restart. The new unit test validates direct assignment to config globals, but production updates go through config::set_config/update_config, where these fields are still immutable CONF_Strings and vector updates are unsupported. The goal is therefore not proven or implemented end-to-end.
- Scope: The code change is small, but it removes the cached filter before the config plumbing supports safe dynamic vector updates.
- Concurrency/lifecycle: The changed recycler scan path is threaded. If these vectors are made truly mutable later, direct unsynchronized iteration over std::vector would also need a safe update/read mechanism or snapshot.
- Config items: Existing config items are central to this PR. They are not mutable, and the runtime setter does not handle std::vectorstd::string, so dynamic changes cannot be applied through the supported path.
- Compatibility/storage/transactions: No incompatible storage, transaction, or persistence format changes found. Recycler two-phase delete/idempotent paths are not otherwise changed.
- Parallel paths: Checker still uses a cached WhiteBlackList. This is not necessarily blocking for the recycler-only goal, but it should be considered if the same config is expected to scope checker behavior dynamically.
- Tests: A unit test was added, but it bypasses the actual runtime config update API and therefore misses the regression this PR is meant to fix. No additional user-provided review focus was supplied.
- Observability/performance: No new observability requirement identified for this small filtering change. Vector scans preserve prior semantics but do not provide the same cached-set lookup characteristics; likely acceptable for small lists.
Please update the runtime config path (and any needed synchronization/snapshotting) so whitelist/blacklist updates can actually be applied without restart, then adjust the test to exercise that supported path.
9d235ba to
236038e
Compare
Collaborator
Author
|
run buildall |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
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.
What problem does this PR solve?
Read recycler whitelist and blacklist directly from config when scanning instances, so runtime config updates can affect filtering without restart.
Add a unit test for dynamic filter changes.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)