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

row_cache: abort on exteral_updater::execute errors #15577

Conversation

bhalevy
Copy link
Member

@bhalevy bhalevy commented Sep 28, 2023

Currently the cache updaters aren't exception safe
yet they are intended to be.

Instead of allowing exceptions from
external_updater::execute escape row_cache::update,
abort using on_fatal_internal_error.

Future changes should harden all execute implementations
to effectively make them noexcept, then the pure virtual
definition can be made noexcept to cement that.

Fixes #15576

ring_position::min() is noexcept since 6d7ae4e
So no need to call it outside of the critical noexcept block.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Currently the cache updaters aren't exception safe
yet they are intended to be.

Instead of allowing exceptions from
`external_updater::execute` escape `row_cache::update`,
abort using `on_fatal_internal_error`.

Future changes should harden all `execute` implementations
to effectively make them `noexcept`, then the pure virtual
definition can be made `noexcept` to cement that.

Fixes scylladb#15576

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
@scylladb-promoter
Copy link
Contributor

@bhalevy
Copy link
Member Author

bhalevy commented Sep 28, 2023

@tgrabiec please review / merge

@bhalevy
Copy link
Member Author

bhalevy commented Oct 31, 2023

@scylladb/scylla-maint please merge

denesb added a commit that referenced this pull request Oct 31, 2023
…y Halevy

Currently the cache updaters aren't exception safe
yet they are intended to be.

Instead of allowing exceptions from
`external_updater::execute` escape `row_cache::update`,
abort using `on_fatal_internal_error`.

Future changes should harden all `execute` implementations
to effectively make them `noexcept`, then the pure virtual
definition can be made `noexcept` to cement that.

Fixes #15576

Closes #15577

* github.com:scylladb/scylladb:
  row_cache: abort on exteral_updater::execute errors
  row_cache: do_update: simplify _prev_snapshot_pos setup
@scylladb-promoter scylladb-promoter merged commit 4a0f164 into scylladb:master Oct 31, 2023
3 checks passed
bhalevy pushed a commit to bhalevy/scylla that referenced this pull request Dec 1, 2023
…y Halevy

Currently the cache updaters aren't exception safe
yet they are intended to be.

Instead of allowing exceptions from
`external_updater::execute` escape `row_cache::update`,
abort using `on_fatal_internal_error`.

Future changes should harden all `execute` implementations
to effectively make them `noexcept`, then the pure virtual
definition can be made `noexcept` to cement that.

\Fixes scylladb#15576

\Closes scylladb#15577

* github.com:scylladb/scylladb:
  row_cache: abort on exteral_updater::execute errors
  row_cache: do_update: simplify _prev_snapshot_pos setup

(cherry picked from commit 4a0f164)
avikivity pushed a commit that referenced this pull request Dec 3, 2023
…y Halevy

Currently the cache updaters aren't exception safe
yet they are intended to be.

Instead of allowing exceptions from
`external_updater::execute` escape `row_cache::update`,
abort using `on_fatal_internal_error`.

Future changes should harden all `execute` implementations
to effectively make them `noexcept`, then the pure virtual
definition can be made `noexcept` to cement that.

\Fixes #15576

\Closes #15577

* github.com:scylladb/scylladb:
  row_cache: abort on exteral_updater::execute errors
  row_cache: do_update: simplify _prev_snapshot_pos setup

(cherry picked from commit 4a0f164)

Closes #16256
denesb added a commit that referenced this pull request Dec 7, 2023
…y Halevy

Currently the cache updaters aren't exception safe
yet they are intended to be.

Instead of allowing exceptions from
`external_updater::execute` escape `row_cache::update`,
abort using `on_fatal_internal_error`.

Future changes should harden all `execute` implementations
to effectively make them `noexcept`, then the pure virtual
definition can be made `noexcept` to cement that.

\Fixes #15576

\Closes #15577

* github.com:scylladb/scylladb:
  row_cache: abort on exteral_updater::execute errors
  row_cache: do_update: simplify _prev_snapshot_pos setup

(cherry picked from commit 4a0f164)

Closes #16256
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.

Row cache updates do not provide strong exception safety guarantees
3 participants