Skip to content

[fix] Prevent stale reads during clear by marking indexes before storage deletion - #141

Merged
0oshowero0 merged 2 commits into
Ascend:mainfrom
dodatboii:fix_clear_index
Jul 29, 2026
Merged

[fix] Prevent stale reads during clear by marking indexes before storage deletion#141
0oshowero0 merged 2 commits into
Ascend:mainfrom
dodatboii:fix_clear_index

Conversation

@dodatboii

Copy link
Copy Markdown
Contributor

Introduce a three-step clear protocol to close the consistency window where storage data is already deleted but the controller still considers the indexes readable:

  1. MARK_CLEARING: zero production_status for the target indexes so consumers cannot fetch them while deletion is in progress
  2. Storage clear: physically remove data from storage units
  3. CLEAR_META / CLEAR_PARTITION: release indexes back to the reusable pool

Signed-off-by: yxstev <zhangyixiang9@huawei.com>
Signed-off-by: yxstev <zhangyixiang9@huawei.com>
@ascend-robot

Copy link
Copy Markdown

CLA Signature Pass

dodatboii, thanks for your pull request. All authors of the commits have signed the CLA. 👍

@dodatboii

Copy link
Copy Markdown
Contributor Author

Solution for #137
cc @0oshowero0 @ji-huazhong

@Chase-Rong

Chase-Rong commented Jul 14, 2026

Copy link
Copy Markdown

Unless I've misunderstood, PR uses production_status = 0 to convey two different meanings:

  • The data has not yet been fully produced
  • The data is being deleted
    But the behavior of these two states is completely different. I think delete operation needs a state that is independent, cannot be overwritten by old requests, and can fail and be retried.

I think we need to modify the state machine to identify the “delete” state separately.
For example

ACTIVE --> PREPARE_CLEAR(clear_id, generation) -- > CLEARING -->delete success --> COMMIT_CLEAR --> DELETED / release index
ACTIVE --> PREPARE_CLEAR(clear_id, generation) -- > CLEARING -->Failed to delete--> keep CLEARING, and retry

@dodatboii

dodatboii commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Unless I've misunderstood, PR uses production_status = 0 to convey two different meanings:

  • The data has not yet been fully produced
  • The data is being deleted
    But the behavior of these two states is completely different. I think delete operation needs a state that is independent, cannot be overwritten by old requests, and can fail and be retried.

I think we need to modify the state machine to identify the “delete” state separately. For example

ACTIVE --> PREPARE_CLEAR(clear_id, generation) -- > CLEARING -->delete success --> COMMIT_CLEAR --> DELETED / release index ACTIVE --> PREPARE_CLEAR(clear_id, generation) -- > CLEARING -->Failed to delete--> keep CLEARING, and retry

Thanks for the thorough analysis.

You're right that production_status = 0 now carries two meanings. However, in practice this ambiguity is safe given the controller's threading model: all requests — including MARK_CLEARING and NOTIFY_DATA_UPDATE — are processed sequentially in a single _process_request loop. There is no concurrent path through which a delayed producer write could race with the clearing sequence and restore production_status to 1 after it has been zeroed.

The full state machine you proposed (PREPARE_CLEAR → CLEARING → COMMIT_CLEAR) would require significant changes to the tensor structure and introduce retry/generation tracking across the client and controller. I'd prefer to keep that as a future improvement if the single-threaded guarantee ever changes, rather than taking on that complexity now.

@Chase-Rong

@Chase-Rong

Copy link
Copy Markdown

Unless I've misunderstood, PR uses production_status = 0 to convey two different meanings:

  • The data has not yet been fully produced
  • The data is being deleted
    But the behavior of these two states is completely different. I think delete operation needs a state that is independent, cannot be overwritten by old requests, and can fail and be retried.

I think we need to modify the state machine to identify the “delete” state separately. For example
ACTIVE --> PREPARE_CLEAR(clear_id, generation) -- > CLEARING -->delete success --> COMMIT_CLEAR --> DELETED / release index ACTIVE --> PREPARE_CLEAR(clear_id, generation) -- > CLEARING -->Failed to delete--> keep CLEARING, and retry

Thanks for the thorough analysis.

You're right that production_status = 0 now carries two meanings. However, in practice this ambiguity is safe given the controller's threading model: all requests — including MARK_CLEARING and NOTIFY_DATA_UPDATE — are processed sequentially in a single _process_request loop. There is no concurrent path through which a delayed producer write could race with the clearing sequence and restore production_status to 1 after it has been zeroed.

The full state machine you proposed (PREPARE_CLEAR → CLEARING → COMMIT_CLEAR) would require significant changes to the tensor structure and introduce retry/generation tracking across the client and controller. I'd prefer to keep that as a future improvement if the single-threaded guarantee ever changes, rather than taking on that complexity now.

@Chase-Rong

ok, that's fine.

@0oshowero0

Copy link
Copy Markdown
Collaborator

We need to add controller side test in later PR

@0oshowero0
0oshowero0 merged commit 76bec91 into Ascend:main Jul 29, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants