Sequence, ANSI, ORACLE and stability fixes #1355
Merged
Conversation
This file contains 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
midenok
force-pushed
the
fixes
branch
4 times, most recently
from
c4636de
to
8f97946
Compare
davidducos
reviewed
Nov 21, 2023
| @@ -974,18 +974,18 @@ commands: | |||
| prepare_ubuntu_percona57: | |||
| steps: | |||
| - run: sudo percona-release setup -y ps57 | |||
| - run: sudo apt-get install -y libperconaserverclient20 percona-server-client-5.7 libperconaserverclient20-dev | |||
| - run: sudo apt-get install -y gdb libperconaserverclient20 percona-server-client-5.7 libperconaserverclient20-dev | |||
There was a problem hiding this comment.
You should not edit the .circleci/config.yml file as it is generated by .circleci/circleci_config_builder.sh
There was a problem hiding this comment.
I changed circleci_config_builder.sh
davidducos
reviewed
Nov 21, 2023
| @@ -399,7 +404,7 @@ int main(int argc, char *argv[]) { | |||
|
|
|||
| struct thread_data t; | |||
| t.thread_id = 0; | |||
| t.conf = &conf; | |||
| t.conf = &conf; // TODO: if conf is singleton it must be accessed as global variable | |||
There was a problem hiding this comment.
On myloader yes! I'm agree with you on this. However, on mydumper it is not.
There was a problem hiding this comment.
This is myloader code. Ok for mydumper.
CREATE TABLE fails if table referencing sequence is restored before sequence. This can happen because sequence restore happens at the very late stage of view restore. To fix this we move sequences restore in between database and table schema restore. We enqueue sequences to buffer queue database::sequence_queue like it is done for tables with database::queue. And we requeue sequence_queue before tables queue in set_db_schema_state_to_created(). We trigger reque like for tables: after database schema was created. When all sequences processed (sequences_processed == sequences) we requeue tables same way in set_db_schema_state_to_created(). There are 2 stages in INTERMEDIATE_ENDED. When first stage entered we wait until all sequences processed and then initiate for all dbs set_db_schema_state_to_created() which requeues their tables into creating queue. INTERMEDIATE_ENDED does loop over all databases. Though conf is the same in all td, according to design set_db_schema_created should access correct td, so we saved it into database struct. Bug description: https://jira.mariadb.org/browse/CONTRIB-25 Fixes mydumper#1338 Sporadic db switch failed fix Race condition: INTERMEDIATE_ENDED may be reached before db schema has been created. INTERMEDIATE_ENDED did set_db_schema_state_to_created() which allowed table schema thread to proceed and it failed because db schema was not yet created. We fix this by waiting INTERMEDIATE_ENDED until each db schema is created. INTERMEDIATE_ENDED does not do requeue now like it did with set_db_schema_state_to_created(), SCHEMA_CREATE does this as it was originally meant to do and signals to INTERMEDIATE_ENDED on cond variable.
Typo fix: datalength index is 5.
There are a lot of useless g_strdup() still, presumably overwriting each others' pointers.
Proper tracing includes thread identifiers borrowed from pthread_t pointers (crafted thread id is not available in every function). Essential for race condition hunting!
INTERMEDIATE_ENDED didn't send JOB_SHUTDOWN properly. It was meant to send JOB_SHUTDOWN to all schema threads, but it turned out it sends all JOB_SHUTDOWN to himself: td was wrongly used. Now we iterate td in the loop across schema_td array.
control_job_thread() initial threads_waiting wrong 0. If INTERMEDIATE_ENDED goes really fast, we call wake_threads_waiting() with 0 which leads to myloader hang.
Related to 14510e3 and https://jira.mariadb.org/browse/CONTRIB-15 g_strsplit() by quote character did wrong if there is quote character inside identifier itself. real_table was made empty string in this case.
midenok
force-pushed
the
fixes
branch
2 times, most recently
from
edeb4db
to
a5c83ca
Compare
midenok
changed the title
midenok
changed the title
midenok
force-pushed
the
fixes
branch
5 times, most recently
from
3725459
to
ce1ca25
Compare
Removed --identifier-quote-character from mydumper, it is now auto-detectected from ANSI or ANSI_QUOTES mode. myloader auto-detects --identifier-quote-character from metadata first matching group name or it may be forced by command-line option. Bug description: https://jira.mariadb.org/browse/CONTRIB-32 Detect quote character v2 (without any metadata changes)
For DROP TABLE we have to set FOREIGN_KEY_CHECKS=0 as well
Like in mysqldump iterate through routine_type to get FUNCTION, PROCEDURE, PACKAGE and PACKAGE BODY. Bug description: https://jira.mariadb.org/browse/CONTRIB-26
NO_TABLE_OPTIONS, NO_TABLE_OPTIONS, NO_FIELD_OPTIONS are dangerous options for mydumper. With NO_TABLE_OPTIONS we miss CHARSET=utf8mb3 from the dump of table schema. Default charset for database is utf8mb4. The schema is so large, so CREATE TABLE under utf8mb4 fails with error: Row size too large. The maximum row size for the used table type, not counting BLOBs, is 65535. But just removing TABLE_OPTIONS, NO_TABLE_OPTIONS, NO_FIELD_OPTIONS from SQL_MODE is not enough. Setting ORACLE automatically adds these to SQL_MODE: PIPES_AS_CONCAT ANSI_QUOTES IGNORE_SPACE NO_KEY_OPTIONS NO_TABLE_OPTIONS NO_FIELD_OPTIONS NO_AUTO_CREATE_USER SIMULTANEOUS_ASSIGNMENT Therefore when dumping table schema we must cut out ORACLE from session variables (as well as other dangerous options). Bug description: https://jira.mariadb.org/browse/CONTRIB-32
Useful for metadata comparison of two dumps.
midenok
force-pushed
the
fixes
branch
5 times, most recently
from
b878275
to
faba7dc
Compare
Fixed: 1. wrong open() status check; 2. wrong directory of metadata.partial.N files; 3. dbt access after free() in process_stream() thread (CONTRIB-10); 4. metadata.partial table could be wrongly detected as metadata file. Stream now first writes metadata.header containing [config] section for proper myloader startup. Findings In stream mode mydumper dumps sql files like usual. After table is dumped metadata_partial_writer() thread gets job to write metadata.partial.N file: a bit of metadata for the table. After metadata bit is written a new job is created for process_stream() which outputs both metadata bit and sql file into STDOUT. In the end we output all databases metadata as the part of usual metadata write (common to non-stream mode). Then we order a job for process_stream() to output that metadata into STDOUT. References: https://jira.mariadb.org/browse/CONTRIB-10
CMake ASAN build fix
In one thread we cannot lock itself on waiting for other thread finishes because there no other thread. Instead we return INTERMEDIATE_ENDED into queue and reiterate. FIXME: merge with 1917e54
give_me_next_data_job_conf() accesses dbt->database->schema_state when dbt is already freed. That happens because main() doesn't wait for control job thread to finish.
davidducos
approved these changes
Nov 27, 2023
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See fix descriptions in commit messages. Sequence fix contans part for dir mode. Stream mode should be fixed as a separate task.
Duplicated by #1341, #1334
Note: there is some setup problem with 2 bionic buildbots.
Example of myloader trace Thread descriptor is added only for --debug option