Skip to content

Fix deletion WAL logging and flush completion ordering - #18872

Merged
jt2594838 merged 2 commits into
apache:masterfrom
hongzhi-gao:fix/delete-wal-null-memtable
Oct 15, 2026
Merged

jt2594838 merged 2 commits into
apache:masterfrom
hongzhi-gao:fix/delete-wal-null-memtable

Conversation

@hongzhi-gao

@hongzhi-gao hongzhi-gao commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Description

A TsFileProcessor is created before table schema registration. If registration fails before the first insert reaches memtable creation, the processor can remain in the working map with a null working memtable. A subsequent tree/table deletion dereferences that null memtable while logging to WAL.

Separately, deletion registration can race with file-close flush completion: after flush scans its queued modifications but before it releases the memtable, a new deletion can be registered and reported as handled in memory. That deletion is never written to the modification file.

Changes

  • Log deletions with the existing MEMTABLE_NOT_EXIST sentinel when the working memtable is null. Retain the WAL entry for IoTConsensus synchronization.
  • Write queued modifications and release the flushed memtable under one write-lock critical section. Preserve the existing deque-monitor-to-write-lock order and existing I/O error handling.
  • Add tree/table WAL serialization coverage for an initially absent working memtable and for a working memtable. Add a synchronized regression through the production asyncClose() path that checks the actual persisted modification file.

Production flush closes the processor and removes it from the working map under the DataRegion write lock. The change preserves the current one-memtable-per-TsFile lifecycle and makes no WAL recovery mapping changes. WAL/RPC formats and configuration remain unchanged.

Validation

mvn spotless:apply -o -pl iotdb-core/datanode
mvn test -o -pl iotdb-core/datanode -am -Dtest=TsFileProcessorTest,WALRecoverManagerTest,UnsealedTsFileRecoverPerformerTest -Dsurefire.failIfNoSpecifiedTests=false
git diff --check

All 35 tests passed, with no failures, errors, or skipped tests. The flush-completion test controls the deque monitor and confirms the flush thread is blocked on that exact monitor before registering deletion, then reads the modification file from disk.

Cluster tests, process-crash fault injection, disk-I/O fault injection, and performance benchmarks were not run.


This PR has:

  • been self-reviewed, including production flush/close callers and concurrent deletion ordering.
  • added comments explaining the intent and lock ordering.
  • added regression tests for deletion WAL logging and flush completion.

Key changed classes
  • TsFileProcessor
  • TsFileProcessorTest

@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 commit 0b7733ded585a39441988358d8ac29e40b9a7946.

No blocking findings from this review. I checked the deletion WAL ID selection for both tree and table nodes, the deletion/flush/checkpoint ordering, and recovery of multiple unflushed memtables in the same TsFile:

  • Holding flushQueryLock through WAL enqueue prevents the selected memtable from being released before the deletion is queued.
  • Writing queued modifications and releasing the memtable under the same write lock closes the late-deletion registration window. The deque monitor is still acquired before the write lock, consistent with syncReleaseFlushedMemTable.
  • All pending memtable IDs for a shared TsFile now route to one recovery performer, with its lifecycle calls deduplicated.

Validation on Windows with JDK 17:

mvn test -o -pl iotdb-core/datanode -am -Dtest=TsFileProcessorTest,WALRecoverManagerTest,UnsealedTsFileRecoverPerformerTest -Dsurefire.failIfNoSpecifiedTests=false -Ddevelocity.off=true
git diff --check 0c0116afe22532f32312a3cb30352a818fba4e38...HEAD

All 36 tests passed, with no failures, errors, or skipped tests, including the new regression cases. Spotless and Checkstyle also passed during the build.

Process-crash and disk-I/O fault injection were not part of this review.

@hongzhi-gao hongzhi-gao changed the title Fix deletion WAL logging and recovery during memtable flush Fix deletion WAL logging and flush completion ordering Oct 15, 2026
@jt2594838
jt2594838 merged commit d77a815 into apache:master Oct 15, 2026
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