Skip to content

Pipe: Fix concurrent TsFile reference increases - #18376

Open
Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/pipe-tsfile-reference-race
Open

Pipe: Fix concurrent TsFile reference increases#18376
Caideyipi wants to merge 1 commit into
apache:masterfrom
Caideyipi:fix/pipe-tsfile-reference-race

Conversation

@Caideyipi

Copy link
Copy Markdown
Collaborator

Description

PipeTsFileResourceManager.increaseFileReference checked whether a resource existed before entering the creation critical section. Historical and realtime extractors could both observe a missing resource, then create and put separate PipeTsFileResource instances for the same hardlink. The later put overwrote the first reference count, so releasing one event could delete a hardlink that another event still needed and stop the pipe with FileNotFoundException.

This change rechecks the resource map after acquiring the segment lock. If another caller has already created the resource, it increments and reuses that resource instead of overwriting it.

A concurrent regression test starts 64 retain operations together and verifies that both the pipe-private and shared reference counts reach 64, then verifies complete cleanup after all releases. Before the fix, the test reproduced a lost count (expected: 64, actual: 58); with the fix, the full test class passes.

Validation:

  • mvn -pl iotdb-core/datanode -Ddevelocity.off=true -Dtest=PipeTsFileResourceManagerTest test
  • mvn -pl iotdb-core/datanode -Ddevelocity.off=true spotless:check
  • mvn -pl iotdb-core/datanode -Ddevelocity.off=true checkstyle:check
  • git diff --check

This PR has:

  • been self-reviewed.
    • concurrent write
  • added comments explaining the why and the intent of the code wherever it would not be obvious.
  • added unit tests to cover the concurrent code path.

Key changed/added classes
  • PipeTsFileResourceManager
  • PipeTsFileResourceManagerTest

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