Skip to content

Import unmarked entities gc sweep dangling oid fix 80 - #738

Merged
hg-ms merged 4 commits into
mainfrom
Import-unmarked-entities-GC-sweep-dangling-OID-fix_80_v2
Jul 7, 2026
Merged

Import unmarked entities gc sweep dangling oid fix 80#738
hg-ms merged 4 commits into
mainfrom
Import-unmarked-entities-GC-sweep-dangling-OID-fix_80_v2

Conversation

@hg-ms

@hg-ms hg-ms commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

A data import (StorageConnection.importFiles / importData) that lands while a storage GC cycle
is active can cause the GC to delete pre-existing entities that the just-imported data durably
references. The corruption is silent at import time; after a restart the storage fails to load —
in the worst case it does not start at all, because root-graph loading is eager
(StorageExceptionConsistency: No entity found for objectId <N>). See #80 for the full analysis.

The root cause is that the import commit path had none of the GC coordination the store path has:
StorageFileManager.commitImport registered imported entities via bare putEntity — no sweep
blocking, no GC-cycle reset, no reference-traversal enqueuing.

This PR coordinates imports with the GC using the same building blocks as the store path, plus one
import-specific measure ("sweep quiescing") for GC cycles that are already in flight when the
import task starts. The mark monitor itself is unchanged.

Semantic contract

An import behaves as if it happened after any sweep cycle that was already initiated when the
import task started.

Two scenarios follow from this, distinguished by when the GC decided about a pre-existing entity
referenced by the imported data:

  1. Referent not yet condemned (mark phase not complete, or the entity is still registered in
    the application's object registry): the import keeps it alive. Imported entities are
    gc-protected and enqueued for reference traversal exactly like stored data, so everything
    reachable through the imported data is marked before any sweep decides what to delete. This
    covers the reported bug (mid-mark window) and application-live referents.
  2. Referent already condemned (a completed mark phase found it unreachable and unregistered,
    only the sweep's execution was still outstanding): the import re-attaches data the GC has
    correctly decided is garbage. Such data is not rescued — deliberately. The result is the
    same, consistent and deterministic, as importing a record that references a long-deleted
    objectId: the condemned subgraph is deleted together and before the import registers
    anything, and the next mark phase reports loud zombie-objectId warnings for the stale
    references. Rescue is provably not implementable here: sparing the directly referenced entity
    only moves the dangling edge one level deeper (its children were condemned by the same
    completed marking), the transitive closure cannot be computed at commit time (entity records
    are channel-owned), and cancelling an in-flight sweep corrupts the mark state in both
    directions. A consistent, loudly-diagnosed deletion is strictly better than a half-rescued
    graph — an import arriving one second later would produce the identical result.

How it works

  • Task-scoped GC signal bracket. Every channel registers a pending store update (blocking new
    sweep initiation) at the very start of the import task's processing phase
    (StorageChannel.prepareImportData()) and releases it only in the task's ultimate cleanUp,
    after commit or rollback. Because the task's commit barrier guarantees all channels have
    registered before the first commit, no channel can initiate a fresh sweep anywhere in the
    cluster while any channel still has unregistered imported entities (this also closes the
    empty-batch-channel gap, where a per-commit bracket would open and close in microseconds).
  • Sweep quiescing. A sweep that was already flagged for a channel when the task started
    executes a marking decision that predates the import — blocking only new sweeps cannot stop
    it. Each channel therefore executes ("quiesces") its own flagged sweep at processing start,
    before any import data is copied or registered. The quiesce is deliberately not gated on the
    GC-enabled debug switch (a flagged sweep belongs to a cycle initiated while GC was enabled;
    leaving it pending would let it run after the commit once GC is re-enabled), and the retry loop
    (the object registry may decline a sweep) is interruptible — an interrupt fails the import
    cleanly instead of hanging shutdown.
  • Gray-enqueue at commit, after durability. commitImport re-arms the GC completion state and
    passes every imported entity through markEntityForChangedData, exactly like stored data —
    since quiescing guarantees no sweep is pending, imported entities always take the gray-enqueue
    path and their references are traversed before the next sweep. The marking pass runs only after
    the transaction-log entry was written: a commit that fails half-way leaves no GC marks behind,
    so the GC never actively traverses a commit that did not complete.

Changes

  • StorageEntityCache: new registerPendingImportUpdate() (signal + completion reset + quiesce
    loop), registerImportCommit() (commit-time completion re-reset, mirroring
    postStorePutEntities), clearPendingImportUpdate(); markEntityForChangedData widened from
    private to package-visible.
  • StorageChannel: prepareImportData() performs the GC registration before switching the file
    manager into import mode; new interface method cleanupImportData() releases it.
  • StorageRequestTaskImportData: prepareImportData() is called outside the shared
    entityCaches lock (a quiesce must not serialize the channels); new task-level cleanUp
    override releases the GC signal on every outcome (commit, rollback, abort).
  • StorageFileManager.commitImport: GC state sync before the registration loop; post-durability
    marking pass over the imported batches.

No changes to StorageEntityMarkMonitor, no new configuration, no observable API change apart
from the added StorageChannel.cleanupImportData() (implemented by the single existing
implementation).

Copilot AI left a comment

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.

Pull request overview

This PR coordinates data imports with the storage garbage collector to prevent a race where an import committing during an active GC cycle can leave durable references to entities that the imminent sweep deletes, causing silent corruption and startup/load failures.

Changes:

  • Adds task-scoped GC coordination for imports (signal bracket held across the whole import task) and import-time sweep quiescing.
  • Ensures import commits reset GC completion state and perform post-durability marking/gray-enqueue of imported entities (mirroring the store path).
  • Extends the import task/channel lifecycle to guarantee GC coordination is released on all outcomes via a new cleanupImportData() hook.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
storage/storage/src/main/java/org/eclipse/store/storage/types/StorageRequestTaskImportData.java Moves prepareImportData() outside the shared entityCaches lock and adds task-level cleanup to always release import GC coordination.
storage/storage/src/main/java/org/eclipse/store/storage/types/StorageFileManager.java Adds import-commit GC coordination and a post-durability marking pass over imported entities to ensure reference traversal before any sweep.
storage/storage/src/main/java/org/eclipse/store/storage/types/StorageEntityCache.java Introduces import-specific GC coordination methods (pending-import signal + sweep quiesce + commit-time reset) and exposes markEntityForChangedData for commit-time use.
storage/storage/src/main/java/org/eclipse/store/storage/types/StorageChannel.java Updates import preparation contract and adds cleanupImportData() to pair with prepareImportData(), implemented in the default channel.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread storage/storage/src/main/java/org/eclipse/store/storage/types/StorageChannel.java Outdated
@hg-ms
hg-ms requested a review from fh-ms July 7, 2026 07:44
@hg-ms
hg-ms merged commit 9741dca into main Jul 7, 2026
16 checks passed
@hg-ms
hg-ms deleted the Import-unmarked-entities-GC-sweep-dangling-OID-fix_80_v2 branch July 7, 2026 08:04
@fh-ms fh-ms added this to the 4.2.0 milestone Jul 28, 2026
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