Skip to content

[refactor](fe) Extract isDynamicScheduleTable method to reduce code duplication#62477

Open
zclllyybb wants to merge 1 commit intoapache:masterfrom
zclllyybb:more_clear_partition
Open

[refactor](fe) Extract isDynamicScheduleTable method to reduce code duplication#62477
zclllyybb wants to merge 1 commit intoapache:masterfrom
zclllyybb:more_clear_partition

Conversation

@zclllyybb
Copy link
Copy Markdown
Contributor

Related PR: #61954

Problem Summary:

Extract the dynamic partition table check logic into a private helper method isDynamicScheduleTable() and add documentation explaining that DynamicPartitionScheduler tracks both dynamic partition tables with scheduling enabled and auto partition tables using partition.retention_count for periodic cleanup.

…uplication

Extract the dynamic partition table check logic into a private helper method `isDynamicScheduleTable()` and add documentation explaining that DynamicPartitionScheduler tracks both dynamic partition tables with scheduling enabled and auto partition tables using `partition.retention_count` for periodic cleanup.
@hello-stephen
Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@zclllyybb
Copy link
Copy Markdown
Contributor Author

run buildall

@zclllyybb
Copy link
Copy Markdown
Contributor Author

/review

Copy link
Copy Markdown
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Apr 14, 2026
@github-actions
Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions
Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking findings.

Critical checkpoint conclusions:

  • Goal and correctness: The PR goal is to extract the duplicated dynamic-schedule table predicate into isDynamicScheduleTable() and document why dynamicPartitionTableInfo contains both dynamic-partition tables and retention-count tables. The updated code still accomplishes that goal: initDynamicPartitionTable() now calls the helper, and the helper preserves the prior condition.
  • Scope and focus: The modification is small, clear, and limited to a single FE refactor in DynamicPartitionScheduler.
  • Concurrency: Applicable but unchanged. The scheduler still evaluates the predicate under table.readLock() in initDynamicPartitionTable(), and the refactor does not introduce new shared-state access or alter locking behavior.
  • Lifecycle/static initialization: No special lifecycle or static initialization changes.
  • Configuration: No new config items and no dynamic-config behavior changes.
  • Compatibility: No protocol, storage format, symbol, or rolling-upgrade compatibility impact.
  • Parallel code paths: I checked the other registration path in DynamicPartitionUtil.registerOrRemoveDynamicPartitionTable(). The helper remains consistent with the scheduler's intended tracked-table set.
  • Special conditional logic: The extracted condition is the same as before; the new Javadoc makes the non-obvious retention-count case easier to understand.
  • Test coverage: No dedicated test was added. For this patch that is acceptable because the behavior is unchanged and the change is a localized extraction/documentation refactor.
  • Test results: No test outputs changed.
  • Observability: No observability changes were needed; existing logs/runtime info remain untouched.
  • Transaction/persistence: Not involved.
  • Data writes/modifications: Not involved.
  • FE-BE variable passing: Not involved.
  • Performance: Neutral to slightly positive for readability/maintainability; no runtime regression identified.
  • Other issues: None found in the reviewed scope.

Residual risk:

  • Low. Because this is a behavior-preserving refactor, the main remaining risk would be an unnoticed semantic mismatch between initialization-time registration and later register/remove paths, but the current helper still matches the effective scheduler semantics I traced in surrounding code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.1.x reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants