Refactor MyRocks iterator status handling #1444
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Contributor
Author
|
As discussed offline, will add comments explaining why two different ways to check RDB iterator status are needed |
- Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature.
laurynas-biveinis
force-pushed
the
myr-iterator-status
branch
from
338d19b to
35e6174
Compare
|
@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing. |
Contributor
Author
|
Unfortunately there is a small conflict with #1445, the second PR to go in will have to be rebased |
|
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
This pull request has been merged in ee2c2fb. |
sunxiayi
pushed a commit
to sunxiayi/mysql-5.6
that referenced
this pull request
Apr 30, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 GitHub Author: Laurynas Biveinis <laurynas.biveinis@gmail.com> Test Plan: Imported from GitHub, without a `Test Plan:` line. Reviewers: herman, chni, #mysql_eng Reviewed By: herman, chni Differential Revision: https://phabricator.intern.facebook.com/D55642013
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
May 16, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
May 17, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
May 17, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
May 21, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
May 21, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
May 21, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
May 30, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
Jul 29, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
Jul 30, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
Jul 31, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
Aug 2, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
inikep
pushed a commit
to inikep/mysql-5.6
that referenced
this pull request
Aug 6, 2024
Summary: - Move is_valid_iterator from ha_rocksdb.cc to rdb_iterator.cc, rename to is_valid_rdb_iterator, make its argument const reference. Remove redundant rdb_persist_corruption_marker call because rdb_handle_io_error calls it too. - Introduce Rdb_iterator_base::convert_iterator_status similar to existing Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic there and additionally handle any other non-I/O errors by saving them in the transaction for a later return. - Make Rdb_iterator_base::convert_get_status take transaction by reference, make it const method. Propagate down the callstack pointer-to-reference changes: rdb_should_hide_ttl_rec, rdb_tx_set_status_error. - Mark touched functions and methods [[nodiscard]] where appropriate. - Annotate rdb_persist_corruption_marker with cold and noinline attributes. - Remove dbug_change_status_to_corrupted, use DBUG_EVALUATE_IF so that the variable gets its final value right at the declaration and could be made const too. This is split out from the range locking feature. Pull Request resolved: facebook#1444 Differential Revision: D55642013 fbshipit-source-id: eeb5d5b
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
is_valid_rdb_iterator, make its argument const reference. Remove redundant
rdb_persist_corruption_marker call because rdb_handle_io_error calls it too.
Rdb_iterator_base::convert_get_status, move HA_ERR_END_OF_FILE-returning logic
there and additionally handle any other non-I/O errors by saving them in the
transaction for a later return.
it const method. Propagate down the callstack pointer-to-reference changes:
rdb_should_hide_ttl_rec, rdb_tx_set_status_error.
variable gets its final value right at the declaration and could be made const
too.
This is split out from the range locking feature.