-
Notifications
You must be signed in to change notification settings - Fork 439
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
One more myloader hang fix + cleanups #1386
Merged
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
Removed useless proxies: give_me_another_non_innodb_chunk_step() give_me_another_innodb_chunk_step() Removed useless hooks in process_queue() Some formatting, some initialization cleanups Comments update_files_on_table_job() cleanup
- Instrumentation via /usr/bin/time; - Backtrace in bash to uderstand where you failed; - Fix Unknown database 'empty_db' in test mydumper#811 (empty_db is dropped between test groups); - Core limit print; - --case/-c option for repeated single case testing; - Diffirent location for sakila-db.tar.gz (MySQL doc site blocks CircleCI for downloading); - Use socket connection when possible (a little bit of refactoring to prettify the code); - MYLOADER_ARGS, MYDUMPER_ARGS env variables pass parameters from the shell.
Client library kind of "supports" some of these environment variables, but this support is buggy and incomplete. F.ex. mysqldump doesn't respect MYSQL_HOST and setting MYSQL_TCP_PORT will not force it to use TCP protocol. We treat this environment in upper layer so we can guarantee and understand their work. --debug enabled for any version. Tracing works ok on earlier versions. Check error status of mysql_select_db().
After INTERMEDIATE_ENDED we may not get any more DATA from refresh_db_queue, therefore we don't push THREAD into refresh_db_queue and don't trigger SHUTDOWN. Good sequence: [DEBUG] - [CJ] Thread control_job_thread started [DEBUG] - [CJ] refresh_db_queue -> THREAD (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue -> THREAD (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue -> THREAD (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue -> INTERMEDIATE_ENDED (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue -> DATA (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue <- THREAD [DEBUG] - [CJ] refresh_db_queue -> THREAD (4 loaders waiting) [DEBUG] - [CJ] here_is_your_job <- SHUTDOWN (5 times) Bad sequence: [DEBUG] - [CJ] Thread control_job_thread started [DEBUG] - [CJ] refresh_db_queue -> THREAD (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue -> THREAD (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue -> DATA (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue -> THREAD (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue -> THREAD (4 loaders waiting) [DEBUG] - [CJ] refresh_db_queue -> INTERMEDIATE_ENDED (4 loaders waiting) In good sequence DATA triggered SHUTDOWN (see "if (intermediate_queue_ended_local && giveup)" in wake_threads_waiting(). In bad sequence DATA doesn't trigger SHUTDOWN as intermediate_queue_ended_local is still false. Probably (only probably) we may omit first wake_threads_waiting(): wake_threads_waiting(conf, &threads_waiting); intermediate_queue_ended_local = TRUE; But it is not guaranteed (I don't understand all the outcomes possible), so it is most safe to keep both calls of wake_threads_waiting().
davidducos
reviewed
Jan 10, 2024
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.
test_mydumper.sh changes is prerequisite for testing integer sharding deferrer. sakila-db is now downloaded from midenok-forks.github.io (but you may set your own github.io site) as downloads.mysql.com blocks circleci (errors was ignored by test_mydumper.sh and it just succeeded on empty base).
Strange enough... As I enabled my own CircleCI testing it stopped to work here (works incorrectly). All jobs passed (several times). Retriggering seems to help.