bb-10.4-galera4 svoj review 4 and 5 fixes - #1078
Merged
svoj merged 16 commits intoJan 15, 2019
Merged
Conversation
janlindstrom
requested changes
Jan 9, 2019
janlindstrom
left a comment
janlindstrom
left a comment
Contributor
There was a problem hiding this comment.
Lets do not remove wsrep plugin.
svoj
requested changes
Jan 9, 2019
| #else | ||
|
|
||
| /* Copyright (c) 2015 MariaDB Corporation Ab | ||
| 2018 Codership Oy <info@codership.com> |
Contributor
There was a problem hiding this comment.
I didn't really mind having Codership here, I was only concerned about GPL header removal.
| @@ -1,7 +1,7 @@ | |||
| # | |||
| !# | |||
Contributor
There was a problem hiding this comment.
I guess it wasn't intentional.
| @@ -97,8 +97,6 @@ extern PSI_cond_key key_COND_prepare_ordered; | |||
| #endif | |||
| #ifdef WITH_WSREP | |||
| int wsrep_thd_binlog_commit(THD *thd, bool all); | |||
Contributor
There was a problem hiding this comment.
wsrep_thd_binlog_commit() is never defined too. Remove it along with TC_LOG_DUMMY::commit()?
| (thd->wsrep_ignore_table == true)) | ||
| { | ||
| return 0; | ||
| } |
Contributor
There was a problem hiding this comment.
We'll get a conflict with PR#1047 here. And again correct way to check for this flag is:
table->file->partition_ht()->flags & HTON_WSREP_REPLICATION
| ticket->wsrep_report(true); | ||
| mysql_mutex_unlock(&granted_thd->LOCK_thd_data); | ||
| ret= TRUE; | ||
| unireg_abort(1); |
| Wsrep_schema_impl::init_stmt(thd); | ||
| if (Wsrep_schema_impl::open_for_write(thd, "members", &members_table)) | ||
| if (Wsrep_schema_impl::open_for_write(thd, cluster_members_str.c_str(), | ||
| &members_table)) |
| { | ||
| if ((wsrep_thd_is_toi(requestor_ctx->get_thd()) || | ||
| wsrep_thd_is_applying(requestor_ctx->get_thd())) && | ||
| key.mdl_namespace() == MDL_key::BACKUP) |
| else | ||
| { | ||
| wsrep_can_grant= FALSE; | ||
| wsrep_handle_mdl_conflict(requestor_ctx, ticket, &key); |
Contributor
There was a problem hiding this comment.
I guess this branch should only be executed by wsrep thread.
Also for performance reasons non-wsrep thread should break immediately after it found incompatible lock.
To avoid code duplication, can we integrate this WITH_WSREP block into original one a few lines below?
Contributor
sjaakola
force-pushed
the
bb-10.4-galera4-svoj-review-4
branch
from
January 13, 2019 11:07
83a0dab to
b74def6
Compare
* brought back the license text in service_wsrep.h, which was dropped by mistake during original merging * introduced new handlerton flag HTON_WSREP_REPLICATION for marking storage engines, which can use wsrep replication provider Refactored to use HTON_WSREP_REPLICATION flag instead of checking if handlerton db_type is DB_TYPE_INNODB * removed the obsolete ha_fake_trx_id usage
* fixed the logic in detecting partition engine * removed duplicate condition in binlog_log_row_internal() (same check already in uper level binlog_log_row())
* galera.partiton test still showing regression reverted back to original wsrep_db_type() method to find out partition engine's underlying table type
* refactored to not use wsrep MDL execption anymore
* fix for the can_grant logic
* wsrep_ignore_table was not dropped from the condition to whetther binlog or not this caused regressions with some tests e.g. galera.galera_gtid_slave
* dropped wsrep_schema and moved tables from there to mysql database many test results affected by renaming
This reverts commit 6362d81.
Previous commit reverted earlier commit: 6362d81 in order to bring back the wsrep_plugin, which was removed in that commit. However, the reverted commit contained also some other changes, which are needed for 10.4. This commit will bring back those reverted changes.
Fixes for comments in the PR#1078
Fix for the MDL can_grant logic, because of wsrep thread processing RSU DDL has wsrep_on==OFF Retained Codership's copyright clause in include/mysql/service_wsrep.h
Fixed a regression which surfaced e.g. with galera.galera_unicode_pk Reason for the regression was that implicit commit for empty transaction did not trigger wsrep transaction cleanup
Removed debug version shortcut, left here by merge mistake
sjaakola
force-pushed
the
bb-10.4-galera4-svoj-review-4
branch
from
January 15, 2019 13:24
325d2d4 to
e1b6ee3
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
This pull request contains code refactoring addressing svoj's reviews 4 and 5