Skip to content

[Pipe] Fix DAL persistence acknowledgments - #18833

Open
Caideyipi wants to merge 1 commit into
masterfrom
codex/fix-page-cache-deletion-dal
Open

Caideyipi wants to merge 1 commit into
masterfrom
codex/fix-page-cache-deletion-dal

Conversation

@Caideyipi

Copy link
Copy Markdown
Collaborator

Description

An oversized first deletion was acknowledged as SUCCESS even when serializeDeletionToBatchBuffer() returned false, leaving no recoverable DAL record. The deletion callers only propagate FAILURE, so they could continue with that false acknowledgment. Both log rotation paths also acknowledged a batch after closing its channel without calling force(false).

Reject an oversized first record through the existing persistence failure path before updating its progress index. Clear failed batch state so a later successful batch cannot acknowledge failed resources. Force the rotated log before closing it and delivering success, and propagate a force failure to the waiting callers. The new error message is provided in both locales.

#18662 is already merged in the base and fixes the close/flush accounting race; these serialization and rotation paths remain affected. Its regression test is retained.

Validation

  • All four new regression tests fail against the original master implementation and pass with this change. They cover an oversized first deletion followed by a recoverable valid deletion, force ordering on both rotation paths, and force failure followed by another batch.

  • Maven: PageCacheDeletionBufferTest, DeletionResourceTest, and DeletionRecoverTest — 13 tests, 0 failures/errors/skips. Run with:

    mvn -pl iotdb-core/datanode -am test \
      -Dtest=PageCacheDeletionBufferTest,DeletionResourceTest,DeletionRecoverTest \
      -DfailIfNoTests=false -Dsurefire.failIfNoSpecifiedTests=false \
      -Diotdb.test.skip=false -Diotdb.ut.skip=false
  • Full reactor English mvn test-compile -DskipTests — passed (53 modules).

  • Full reactor Chinese mvn test-compile -P with-zh-locale -DskipTests — passed (53 modules).

  • mvn -pl iotdb-core/datanode spotless:check checkstyle:check — passed; git diff --check is clean. The locale message keys and format specifiers match.


This PR has:

  • been self-reviewed.
  • added comments explaining the intent of the code.
  • added unit tests covering the affected code paths.
Key changed classes

PageCacheDeletionBuffer, PageCacheDeletionBufferTest, and DataNodePipeMessages (en/zh).

@codecov

codecov Bot commented Oct 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.92%. Comparing base (932c1da) to head (d27d37b).

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18833      +/-   ##
============================================
+ Coverage     45.91%   45.92%   +0.01%     
  Complexity      712      712              
============================================
  Files          5502     5502              
  Lines        398775   398779       +4     
  Branches      51967    51969       +2     
============================================
+ Hits         183080   183124      +44     
+ Misses       215695   215655      -40     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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