Prep for 8.0 #930
Open
Prep for 8.0 #930
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
added 8 commits
January 11, 2019 11:13
…cording to MySQL standards. - Changed all includes of mysql headers to use "" instead of <> and a little movement of some that were commented incorrectly. - 5.6 and 5.7 have some server paths within the INCLUDE path and their standard is to NOT include these as part of the #include directve (specifically "sql"). 8.0 restructures the include scheme significantly and will remove "sql" from the INCLUDE path, moves files areound, factors out new headers, and generally implements an "include what you use" scheme. This will require many #include directives to be updated to use the new system.
…red_map - In prep for 8.0, removed MySQL HASH and replaced with much simpler std::unordered_map. 8.0 re-implements HASH and forces re-write of any code that uses. For the purpose of Rdb_open_tables_map, it is unneeded and can easily be replaced with std::unordered_map.
- In prep for 8.0, removed MySQL HASH and replaced with much simpler std::unordered_map. 8.0 re-implements HASH and forces re-write of any code that uses. For the purpose of Rdb_open_tables_map, it is unneeded and can easily be replaced with std::unordered_map.
… rwlock keys with PSI - Changed use of PSI_server->register_[mutex|rwlock] to mysq_[mutex|rwlock]_register - Corrected/cleaned psi includes
…ing my_checksum anyway - Converted direct calls to zlib crc32 to use my_checksum which also calls zlib crc32. This is because the forward declaration of crc32 was removed from my_sys.h in 8.0 and it is generally good practice to use my_ calls.
- MyRocks relies on server code Field_*::make_sort_key to generate key data that it will persist. The problem is that the results of these methods are/were never intended to be persisted and the results of these methods may change over time. When they do they break MyRocks ability decode its own key data that it has previously stored. This has now happened in 8.0 as of 8.0.11 the returned format for FLOAT and DOUBLE is different than is for 5.6 and 5.7. This breaks upgrades and also breaks MyRocks ability to encode, store, retrieve, and decode keys of type FLOAT and DOUBLE. - In order to prevent this from happening again in 5.7 and 8.0, this commit copies the current logic from Field_*::make_sort_key into the MyRocks data dictionary code, thus breaking the MyRocks dependency on the Field_*::make_sort_key functionality. Only fixed length CHAR types remain using the make_sort_key method.
- Changed conditional compilation for function used as part of DBUG_EXECUTE_IF from NDEBUG to DBUG_OFF. This causes issues in 8.0 and I do believe it is wrong to use NDEBUG when attempting any kind of conditional compilation that interacts with my_dbug.h. All functionality within my_dbg.h is based on !DBUG_OFF, therefore, any use of my_dbug.h and external conditional compilation in support of its use should use the same base indicator (DBUG_OFF) and not rely on an assumption that NDEBUG == DBUG_OFF because they are not the same.
…/write_batch_with_index.cc:212: void rocksdb::BaseDeltaIterator::Advance(): Assertion `BaseValid()' failed. - Issue appears only in 8.0 due to the changes in index use during the handling of a row deletion event on a slave when binlog_format=row, but, the MyRocks fix still holds true for 5.6 and 5.7. - Core issue seems to be that while the slave is in slave_exec_mode=IDEMPOTENT mode, and encounters an HA_ERR_KEY_NOT_FOUND when looking for a specific record that is supposed to be deleted, it correctly ignores it, but, the index scan state is now 'invalid' or undefined. The sequence in 8.0 is played out in log_event.cc Rows_log_event::do_scan_and_update(...). For the row that is to be deleted in the replication event, the handler calls index_read_map() to position the index, then delete_row(), then index_next_same(). Since MyRocks does not implement index_next_same(), the handler default method is invoked which in turn calls MyRocks index_next(). This re-positioning is to find duplicate keys for deletion. When index_read_map() returns the HA_ERR_KEY_NOT_FOUND, the delete_row() call is skipped, but, the index_next_same() is still called. The problem in MyRocks when the row is not found is that index_read_map() leaves the RocksDB scan iterator in an 'invalid' state, which means it is unpositioned. RocksDB asserts if there is any attempt to move an unpositioned iterator via a Prev() or Next() type scanning call. So, when index_read_map() invalidates the iterator, and then index_next() is subsequently called, RocksDB asserts within the Next() call. I believe the fix that is needed is in reality in 8.0 on the server side in that it should not be advancing on an unpositioned index (since index_read_map() failed I believe by definition, the index is not positioned). That said, MyRocks should also be a little more responsible and validate that an index is positioned before attempting to use any of the scanning calls (Prev() and Next()). This commit handles exactly that and returns HA_ERR_KEY_NOT_FOUND from index_next_with_direction() if it is called and there is an unpositioned iterator.
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.
A series of mostly non functional changes to begin aligning the 5.6 code base (and 5.7 via Percona Server) with changes made during port to 8.0.