Skip to content

Pipe: improve progress coverage checks#17940

Merged
jt2594838 merged 3 commits into
apache:masterfrom
Caideyipi:fix-pipe-progress-coverage
Jun 16, 2026
Merged

Pipe: improve progress coverage checks#17940
jt2594838 merged 3 commits into
apache:masterfrom
Caideyipi:fix-pipe-progress-coverage

Conversation

@Caideyipi

Copy link
Copy Markdown
Collaborator

Description

Progress coverage

Add ProgressIndex.isEqualOrAfter(...) and use it for pipe progress coverage checks. Historical TsFile selection now checks whether the start index already covers the resource progress by reusing updateToMinimumEqualOrIsAfterProgressIndex, which handles Hybrid/Recover partial-dimension cases consistently.

Diagnostics

Add historical TsFile selection summary counters for progress-uncovered selection, unclosed/closing selection, time/path filtering, covered skips, deleted/generated-by-pipe skips, and pin failures. Shutdown progress persistence now logs pipe/meta counts, meta size, heartbeat timing, and reports clearly when shutdown progress is not confirmed by ConfigNode before the deadline.

Tests

Add Hybrid/Recover coverage cases for historical TsFile selection and realtime TsFile epoch progress checks.

Validation:

  • .\mvnw.cmd -Ddevelocity.off=true -pl iotdb-core/node-commons,iotdb-core/datanode spotless:apply
  • git diff --check origin/master

Targeted tests were attempted locally, but DataNode test compilation is currently blocked by unrelated existing errors in generated query/parser and queue classes.


This PR has:

  • been self-reviewed.
  • added Javadocs for most classes and all non-trivial methods.
  • added comments explaining the why and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage.

Key changed/added classes (or packages if there are too many classes) in this PR
  • ProgressIndex
  • PipeHistoricalDataRegionTsFileAndDeletionSource
  • PipeTsFileEpochProgressIndexKeeper
  • PipeDataNodeTaskAgent
  • DataNodeShutdownHook

Comment on lines +134 to +135
private static final String SHUTDOWN_PROGRESS_NOT_CONFIRMED_MESSAGE =
"\u672c\u6b21 shutdown progress \u672a\u786e\u8ba4\u843d\u5230 ConfigNode.";

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.

What is this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. Removed the hard-coded Chinese suffix and moved the shutdown progress confirmation message into DataNodePipeMessages for both en/zh locales.

Comment on lines +646 to +649
LOGGER.info(
"Start to persist all pipe progress indexes during shutdown, pipe count {}, deadline {} ms",
getPipeCount(),
normalizedTimeoutInMs);

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.

i18n, the same below.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. All newly added shutdown progress logs now use DataNodePipeMessages, with English and Chinese i18n entries.

Comment on lines +714 to +717
LOGGER.warn(
"Collected empty pipe metas for {} pipes during shutdown. "
+ SHUTDOWN_PROGRESS_NOT_CONFIRMED_MESSAGE,
pipeCount);

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.

Necessary to warn?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. Changed this log to info. The method still returns false when pipeCount is non-zero but no meta is collected, so the shutdown hook keeps the single final warning that progress was not confirmed.

Comment on lines 757 to +762
} catch (final Exception e) {
LOGGER.warn(e.getMessage());
LOGGER.warn(
"Exception occurred while persisting all pipe progress indexes during shutdown. "
+ SHUTDOWN_PROGRESS_NOT_CONFIRMED_MESSAGE,
e);
return false;

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.

May ignore interruption here

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. The join interruption now restores the interrupt flag and logs at info without interrupting the daemon persist thread.

@jt2594838 jt2594838 merged commit b332786 into apache:master Jun 16, 2026
59 of 62 checks passed
@jt2594838 jt2594838 deleted the fix-pipe-progress-coverage branch June 16, 2026 09:11
jt2594838 pushed a commit that referenced this pull request Jun 18, 2026
* Pipe: improve progress coverage checks (#17940)

* Pipe: improve progress coverage checks

* Pipe: address shutdown progress review comments

* Pipe: refine Chinese shutdown progress messages

(cherry picked from commit b332786)

* Fix Java 8 test compatibility
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.

2 participants