Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

database::drop_column_family(): has a window where new queries can be created for the dropped cf #10450

Closed
denesb opened this issue Apr 28, 2022 · 1 comment · Fixed by #10451
Assignees
Labels
Milestone

Comments

@denesb
Copy link
Contributor

denesb commented Apr 28, 2022

When such an entry expire, it will cause a crash because destroying the reader objects requires the table object to be around (which obviously isn't after the drop).
https://github.com/scylladb/scylla/blob/333fdcb3f544a6fc2ef3066e2ddbf1bde18cb7a1/replica/database.cc#L930-L936

Even though all querier cache entries related to the to-be-dropped table are removed by database::remove(), a read running concurrent to database::drop_column_family() can create a new querier, when said method yields to wait for ongoing operations.

@denesb denesb changed the title database::drop_column_family(): has a window where queries can be created for the dropped cf, left to expire database::drop_column_family(): has a window where new queries can be created for the dropped cf Apr 28, 2022
@denesb denesb self-assigned this Apr 28, 2022
avikivity added a commit that referenced this issue May 1, 2022
… querier cache entries' from Botond Dénes

Said method has to evict all querier cache entries, belonging to the to-be-dropped table. This is already the case, but there was a window where new entries could sneak in, causing a stale reference to the table to be de-referenced later when they are evicted due to TTL. This window is now closed, the entries are evicted after the method has waited for all ongoing operations on said table to stop.

Fixes: #10450

Closes #10451

* github.com:scylladb/scylla:
  replica/database: drop_column_family(): drop querier cache entries after waiting for ops
  replica/database: finish coroutinizing drop_column_family()
  replica/database: make remove(const column_family&) private

(cherry picked from commit 7f1e368)
avikivity added a commit that referenced this issue May 1, 2022
… querier cache entries' from Botond Dénes

Said method has to evict all querier cache entries, belonging to the to-be-dropped table. This is already the case, but there was a window where new entries could sneak in, causing a stale reference to the table to be de-referenced later when they are evicted due to TTL. This window is now closed, the entries are evicted after the method has waited for all ongoing operations on said table to stop.

Fixes: #10450

Closes #10451

* github.com:scylladb/scylla:
  replica/database: drop_column_family(): drop querier cache entries after waiting for ops
  replica/database: finish coroutinizing drop_column_family()
  replica/database: make remove(const column_family&) private

(cherry picked from commit 7f1e368)
avikivity added a commit that referenced this issue May 1, 2022
… querier cache entries' from Botond Dénes

Said method has to evict all querier cache entries, belonging to the to-be-dropped table. This is already the case, but there was a window where new entries could sneak in, causing a stale reference to the table to be de-referenced later when they are evicted due to TTL. This window is now closed, the entries are evicted after the method has waited for all ongoing operations on said table to stop.

Fixes: #10450

Closes #10451

* github.com:scylladb/scylla:
  replica/database: drop_column_family(): drop querier cache entries after waiting for ops
  replica/database: finish coroutinizing drop_column_family()
  replica/database: make remove(const column_family&) private

(cherry picked from commit 7f1e368)
avikivity added a commit that referenced this issue May 1, 2022
… querier cache entries' from Botond Dénes

Said method has to evict all querier cache entries, belonging to the to-be-dropped table. This is already the case, but there was a window where new entries could sneak in, causing a stale reference to the table to be de-referenced later when they are evicted due to TTL. This window is now closed, the entries are evicted after the method has waited for all ongoing operations on said table to stop.

Fixes: #10450

Closes #10451

* github.com:scylladb/scylla:
  replica/database: drop_column_family(): drop querier cache entries after waiting for ops
  replica/database: finish coroutinizing drop_column_family()
  replica/database: make remove(const column_family&) private

(cherry picked from commit 7f1e368)
@avikivity
Copy link
Member

Backported to 4.5, 4.6, 5.0.

avikivity added a commit that referenced this issue May 1, 2022
… querier cache entries' from Botond Dénes

Said method has to evict all querier cache entries, belonging to the to-be-dropped table. This is already the case, but there was a window where new entries could sneak in, causing a stale reference to the table to be de-referenced later when they are evicted due to TTL. This window is now closed, the entries are evicted after the method has waited for all ongoing operations on said table to stop.

Fixes: #10450

Closes #10451

* github.com:scylladb/scylla:
  replica/database: drop_column_family(): drop querier cache entries after waiting for ops
  replica/database: finish coroutinizing drop_column_family()
  replica/database: make remove(const column_family&) private

(cherry picked from commit 7f1e368)
@DoronArazii DoronArazii added this to the 5.0 milestone Jul 7, 2022
@DoronArazii DoronArazii modified the milestones: 5.0, 5.1 Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants