Skip to content

Fix tier folder recovery and DiskFull status after configuration reload - #19161

Merged
jt2594838 merged 7 commits into
apache:masterfrom
hongzhi-gao:fix/tier-manager-full-disk-recovery
Nov 10, 2026
Merged

jt2594838 merged 7 commits into
apache:masterfrom
hongzhi-gao:fix/tier-manager-full-disk-recovery

Conversation

@hongzhi-gao

@hongzhi-gao hongzhi-gao commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Problem

When every directory managed by a FolderManager is full during DataNode startup or configuration reload, construction fails. TierManager previously skipped that manager in its tier lists. Freeing disk space could then restore the DataNode to Running, but a later TsFile allocation could still fail with IndexOutOfBoundsException or select the wrong tier because the list no longer matched the configured tier levels.

A successful configuration reload could also clear ReadOnly(DiskFull) even when the same directories remained full or another disk-full source had not recovered.

Changes

  • Keep an entry for each configured tier and directory type before constructing its FolderManager. Retry a failed construction on the next access and reuse successfully constructed managers.
  • Register recovery checks against the current directories for tiers, snapshot loading, LOAD staging, and active LOAD listening. A directory reload or a new disk-full event invalidates recovery results obtained before that change; independent checks such as WAL remain in force.
  • Remove the unconditional transition to Running after a successful reload. Heartbeat recovery now requires both the aggregate disk-space check and every registered disk-full recovery check to pass.

Verification

  • Focused DataNode and node-commons unit/component tests passed, including initialization while full, retry after recovery, a failed snapshot manager followed by an added data directory, an unrelated WAL disk-full source, and concurrent recovery checks. Existing LOAD and snapshot component tests also passed.
  • python3 .github/scripts/check-it-selection.py passed.
  • mvn spotless:apply -o -pl integration-test -P with-integration-tests -Ddevelocity.off=true passed.
  • mvn verify -o -DskipUTs '-Dit.test=IoTDBDiskFullFolderRecoveryIT' -DfailIfNoTests=false -Dfailsafe.failIfNoSpecifiedTests=false -pl integration-test -am -P with-integration-tests -Ddevelocity.off=true passed: 1 test, 0 failures, 0 errors, 0 skipped. The IT verifies ReadOnly(DiskFull) during a full-directory reload, continued read-only status after an unchanged reload, recovery to Running, and successful TsFile creation and query.

The IT raises the disk-space warning threshold to exercise the real DataNode allocation and recovery paths without physically filling the host filesystem.

@hongzhi-gao hongzhi-gao closed this Nov 6, 2026
@hongzhi-gao hongzhi-gao changed the title Fix tier folder allocation after disk-full initialization Fix tier folder recovery and DiskFull status after configuration reload Nov 6, 2026
@hongzhi-gao hongzhi-gao reopened this Nov 6, 2026

@Caideyipi Caideyipi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 378f1a0. Found one recovery regression involving discarded FolderManagers that captured data directories before a configuration reload. A focused regression test successfully selected a TsFile folder on the added directory, then failed the recovery assertion: expected Running, actual ReadOnly (1 test, 1 failure, 0 errors).

}
reloadMetricProperties(commonProperties);
if (wasReadOnlyForDiskFull) {
commonConfig.setRunningIfDiskFull();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Refresh non-tier data-directory recovery checks on reload

With this clearing step removed, checks from ordinary FolderManagers also survive a successful reload, but only TierManager checks follow the new configuration. For example, SnapshotLoader.createLinksFromSnapshotDirToDataDirWithoutLog() constructs a plain FolderManager from getLocalDataDirs(). If construction fails while the old data disk is full, DirectoryStrategy.setFolders() registers a callback over List.copyOf(oldDirs). After dn_data_dirs adds a free disk, new tier and snapshot-loading managers can use that disk, while the discarded snapshot manager's callback still checks only the full old disk. Even after the aggregate disk-space check passes, tryRecoverFromDiskFull() therefore keeps the whole DataNode in ReadOnly(DiskFull). Please refresh or retire checks owned by these reloadable data-directory consumers as well, while preserving independent sources such as WAL. A regression should cover failed snapshot-manager construction, adding a free data directory through reload, and subsequent heartbeat recovery.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good catch. Fixed in cfd1dce: SnapshotLoader, LoadStagingDirs, and LoadUtil now check their current configured directories for DiskFull recovery, while independent sources such as WAL remain registered. The new SnapshotLoaderDiskFullReloadTest covers failed snapshot FolderManager construction, adding a writable data directory, and the heartbeat recovery check (tryRecoverFromDiskFull). It also verifies that an unavailable WAL source still keeps the node ReadOnly. The focused tests passed.

Comment on lines +319 to +324
private FolderManager getFolderManager(int tierLevel, String folderName)
throws DiskSpaceInsufficientException {
TierFolderManager entry = folderManagers.get(new TierDirectory(tierLevel, folderName));
if (entry == null) {
throw new IndexOutOfBoundsException(tierLevel);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Provide both the tierLevel and the folderName in the exception.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in cfd1dce. The missing-key exception now includes the TierDirectory key, whose message contains both tierLevel and folderName (for example, TierDirectory[tierLevel=2, folderName=sequence]). TierManagerTest asserts both values.

@jt2594838
jt2594838 merged commit d10b69e into apache:master Nov 10, 2026
38 of 39 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.

3 participants