Skip to content

Refactor node status recovery from DiskFull - #18873

Closed
d-wang-commit wants to merge 1 commit into
apache:masterfrom
d-wang-commit:refactor-disk-full-status-recovery
Closed

d-wang-commit wants to merge 1 commit into
apache:masterfrom
d-wang-commit:refactor-disk-full-status-recovery

Conversation

@d-wang-commit

Copy link
Copy Markdown
Contributor

Description

Centralize disk recovery in CommonConfig

Disk recovery was implemented in DataNodeInternalRPCServiceImpl.sampleDiskLoad(), while node status and reason updates were managed by CommonConfig. Move the guarded recovery into CommonConfig.recoverFromDiskFull() and share state assignment and transition logging in a private updateNodeStatus() method.

Recovery still applies only to ReadOnly/DiskFull. It also checks the stopping flag under the same monitor used by status updates and setStopping(). This closes the shutdown interval between setting the stopping flag and assigning the Stopping reason, during which a healthy disk sample could previously restore Running.

Tests

  • UT: add CommonConfigTest.testDiskRecoveryDoesNotUndoLaterStatusChange and DataNodeInternalRPCServiceImplDiskTest.testStoppingFlagPreventsDiskRecovery. Existing tests also check disk thresholds and preservation of Manual, UnrecoverableError and Stopping reasons. Both test classes passed: 18 tests, no failures or errors.
  • Build: the full reactor package build with with-zh-locale and tests skipped passed.
  • IT: no integration tests added or run; no live-cluster validation performed.

Side effects and risks

Disk recovery now intentionally leaves the node read-only once shutdown has started. The stopping setter acquires the existing configuration monitor, serializing it with disk recovery and status updates. The critical section is limited to the state transition and its existing logging. No configuration, storage-format or RPC changes are introduced; read-only reason priorities remain unchanged.


This PR has:

  • been self-reviewed.
    • concurrent read
    • concurrent write
    • concurrent read and write
  • added Javadocs for the new non-trivial methods.
  • added unit tests for the new recovery entry point and the shutdown interval.

Key changed/added classes (or packages if there are too many classes) in this PR
  • CommonConfig: owns synchronized disk recovery and serializes it with the stopping flag.
  • DataNodeInternalRPCServiceImpl: delegates healthy-disk recovery to CommonConfig.
  • CommonConfigTest, DataNodeInternalRPCServiceImplDiskTest: cover later status changes and recovery during shutdown.

Move disk recovery checks into CommonConfig and share status updates.
Keep recovery limited to ReadOnly/DiskFull and block it once shutdown starts.
Add regression tests for shutdown and later status changes.
@d-wang-commit
d-wang-commit marked this pull request as draft October 14, 2026 10:15
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.

1 participant