Skip to content

Fix: Invalidate tablets when table is dropped via schema event - #975

Open
mykaul wants to merge 1 commit into
scylladb:masterfrom
mykaul:fix/drop-table-event-invalidate-tablets
Open

Fix: Invalidate tablets when table is dropped via schema event#975
mykaul wants to merge 1 commit into
scylladb:masterfrom
mykaul:fix/drop-table-event-invalidate-tablets

Conversation

@mykaul

@mykaul mykaul commented Aug 11, 2026

Copy link
Copy Markdown

Issue

The event-driven DROP TABLE path (Metadata._drop_table) never drops tablet metadata. Tablets are only invalidated on full schema rebuilds, keyspace events, and host removal. Stale tablet entries remain after a table is dropped, and a dropped-then-recreated table briefly routes on stale tablet data.

Fix

Call the existing _table_removed() helper from _drop_table(), outside the try block so invalidation also fires when the keyspace is unknown (race, or schema metadata disabled).

Test

Unit: Metadata._drop_table invalidates tablets for known and unknown keyspaces.
Integration: test_tablets_invalidation_drop_table drops a tablet table and waits for the tablet record to be purged.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Metadata._drop_table now removes tablet metadata after removing table or view metadata. Unit tests cover known and unknown keyspaces. An integration test verifies asynchronous tablet invalidation after dropping a dedicated table.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies tablet invalidation when a table is dropped through a schema event.
Description check ✅ Passed The description explains the issue, fix, and unit and integration tests, but it does not include the repository checklist.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
tests/integration/standard/test_tablets.py-221-234 (1)

221-234: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Make table restoration unconditional and reuse the class session.

If run_tablets_invalidation_test(drop_table) raises, the recreation block is skipped. Later tests then run without test1.table1. Put restoration in a finally block or cleanup hook. Use self.session instead of opening sessions that are not explicitly closed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/standard/test_tablets.py` around lines 221 - 234, Update
test_tablets_invalidation_drop_table so table1 restoration always runs in a
finally block, including when run_tablets_invalidation_test raises. Reuse
self.session for both the DROP TABLE callback and the CREATE TABLE/data setup,
removing the temporary cluster.connect() sessions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/test_metadata.py`:
- Around line 445-465: Add concise docstrings to DropTableMetadataTest, its
setUp method, and both test_drop_table_invalidates_tablets methods in
tests/unit/test_metadata.py (lines 445-465). Also add a docstring to
test_tablets_invalidation_drop_table in
tests/integration/standard/test_tablets.py (lines 221-226), describing each
test’s purpose.

---

Other comments:
In `@tests/integration/standard/test_tablets.py`:
- Around line 221-234: Update test_tablets_invalidation_drop_table so table1
restoration always runs in a finally block, including when
run_tablets_invalidation_test raises. Reuse self.session for both the DROP TABLE
callback and the CREATE TABLE/data setup, removing the temporary
cluster.connect() sessions.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Pro Plus

Run ID: 48fe95f3-28e6-4a63-bdc3-0ffec5109391

📥 Commits

Reviewing files that changed from the base of the PR and between e9773cd and 600f53a.

📒 Files selected for processing (3)
  • cassandra/metadata.py
  • tests/integration/standard/test_tablets.py
  • tests/unit/test_metadata.py

Comment thread tests/unit/test_metadata.py
@mykaul
mykaul force-pushed the fix/drop-table-event-invalidate-tablets branch from 600f53a to c569402 Compare August 11, 2026 10:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
tests/integration/standard/test_tablets.py-228-236 (1)

228-236: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the restoration cleanup idempotent.

run_tablets_invalidation_test can fail before drop_table runs, such as when it cannot find a tablet record at Line 280. The finally block then creates test1.table1 while it still exists, which can raise AlreadyExists and hide the original failure. Use CREATE TABLE IF NOT EXISTS or preserve the active test exception during cleanup.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/standard/test_tablets.py` around lines 228 - 236, Make the
restoration cleanup in the test method containing run_tablets_invalidation_test
idempotent by changing its table recreation statement to use CREATE TABLE IF NOT
EXISTS. Keep create_data(self.session) in the finally block so the shared table
data is restored without masking the original test failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Other comments:
In `@tests/integration/standard/test_tablets.py`:
- Around line 228-236: Make the restoration cleanup in the test method
containing run_tablets_invalidation_test idempotent by changing its table
recreation statement to use CREATE TABLE IF NOT EXISTS. Keep
create_data(self.session) in the finally block so the shared table data is
restored without masking the original test failure.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Pro Plus

Run ID: fe4a0215-f2cb-49b4-9f72-7ef065d9b729

📥 Commits

Reviewing files that changed from the base of the PR and between 600f53a and c569402.

📒 Files selected for processing (2)
  • tests/integration/standard/test_tablets.py
  • tests/unit/test_metadata.py

@mykaul
mykaul force-pushed the fix/drop-table-event-invalidate-tablets branch from c569402 to 1a4645a Compare August 11, 2026 12:07
Signed-off-by: Yaniv Michael Kaul <yaniv.kaul@scylladb.com>
@mykaul
mykaul force-pushed the fix/drop-table-event-invalidate-tablets branch from 1a4645a to e154dec Compare August 11, 2026 12:13

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
tests/integration/standard/test_tablets.py-279-284 (1)

279-284: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Assert cleanup for every tablet record.

run_tablets_invalidation_test checks only the record for pk=2. The new test inserts 50 rows, so stale records for other tablets can remain undetected. Collect records for all inserted keys, or assert that the dropped table has no remaining tablet entries.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/standard/test_tablets.py` around lines 279 - 284, Update
run_tablets_invalidation_test to validate tablet metadata cleanup for every
inserted key rather than only pk=2. Collect or query records across all 50
inserted keys, and assert that no tablet entries remain for the dropped table
while preserving the existing invalidation flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Other comments:
In `@tests/integration/standard/test_tablets.py`:
- Around line 279-284: Update run_tablets_invalidation_test to validate tablet
metadata cleanup for every inserted key rather than only pk=2. Collect or query
records across all 50 inserted keys, and assert that no tablet entries remain
for the dropped table while preserving the existing invalidation flow.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Pro Plus

Run ID: 772fc231-b3f3-4a7c-a50d-0a78f6c5fc28

📥 Commits

Reviewing files that changed from the base of the PR and between 1a4645a and e154dec.

📒 Files selected for processing (1)
  • tests/integration/standard/test_tablets.py

@mykaul mykaul changed the title Invalidate tablets when table is dropped via schema event Fix: Invalidate tablets when table is dropped via schema event Aug 11, 2026
@mykaul

mykaul commented Aug 11, 2026

Copy link
Copy Markdown
Author

Other comments:
In @tests/integration/standard/test_tablets.py:

  • Around line 279-284: Update run_tablets_invalidation_test to validate tablet
    metadata cleanup for every inserted key rather than only pk=2. Collect or query
    records across all 50 inserted keys, and assert that no tablet entries remain
    for the dropped table while preserving the existing invalidation flow.

Nope, I don't think it's needed at all, rabbit.

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