MDEV-33087 ALTER TABLE...ALGORITHM=COPY should build indexes more efficiently #3145
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
|
|
dr-m
reviewed
Mar 21, 2024
Thirunarayanan
force-pushed
the
10.11-MDEV-33087
branch
4 times, most recently
from
f0ea2cf
to
fdf922b
Compare
Thirunarayanan
force-pushed
the
10.11-MDEV-33087
branch
from
fdf922b
to
68ac097
Compare
dr-m
reviewed
Apr 3, 2024
There was a problem hiding this comment.
Here are some initial comments. I think that MDEV-33809 needs to be fixed before this can be merged.
storage/innobase/row/row0ins.cc
Outdated
Comment on lines
3401
to
3414
| /* MDEV-25036 FIXME: check also foreign key | ||
| constraints */ | ||
| ut_ad(!trx->check_foreigns); | ||
| ut_ad(index->table->skip_alter_undo | ||
| || !trx->check_foreigns); |
There was a problem hiding this comment.
It would be good to have a comment that explains where the FOREIGN KEY constraints will be enforced during ALTER TABLE…ALGORITHM=COPY.
There was a problem hiding this comment.
I think that a source code comment here about the FOREIGN KEY constraint enforcement would be useful. In which function is that check executed?
Thirunarayanan
force-pushed
the
10.11-MDEV-33087
branch
2 times, most recently
from
f5dabc8
to
c4b52b0
Compare
dr-m
reviewed
Apr 12, 2024
Thirunarayanan
force-pushed
the
10.11-MDEV-33087
branch
2 times, most recently
from
0baa8f6
to
a6bb0ca
Compare
dr-m
reviewed
Apr 22, 2024
Thirunarayanan
force-pushed
the
10.11-MDEV-33087
branch
from
a6bb0ca
to
af9d45b
Compare
dr-m
reviewed
Apr 25, 2024
storage/innobase/row/row0ins.cc
Outdated
Comment on lines
3401
to
3414
| /* MDEV-25036 FIXME: check also foreign key | ||
| constraints */ | ||
| ut_ad(!trx->check_foreigns); | ||
| ut_ad(index->table->skip_alter_undo | ||
| || !trx->check_foreigns); |
There was a problem hiding this comment.
I think that a source code comment here about the FOREIGN KEY constraint enforcement would be useful. In which function is that check executed?
Thirunarayanan
force-pushed
the
10.11-MDEV-33087
branch
2 times, most recently
from
cbaffd3
to
06ef346
Compare
dr-m
reviewed
May 2, 2024
Thirunarayanan
force-pushed
the
10.11-MDEV-33087
branch
2 times, most recently
from
dd1e145
to
1f66c6c
Compare
dr-m
reviewed
May 10, 2024
mysql-test/suite/innodb/r/bulk_copy_alter,non_bulk_alter_copy.rdiff
Outdated
Show resolved
Hide resolved
…iciently - During copy algorithm, InnoDB should use bulk insert operation for row by row insert operation. By doing this, copy algorithm can effectively build indexes. This optimization is disabled for temporary table, versioning table and table which has foreign key relation. Introduced the variable innodb_alter_copy_bulk to allow the bulk insert operation for copy alter operation inside InnoDB. This is enabled by default ha_innobase::extra(): HA_EXTRA_END_ALTER_COPY mode tries to apply the buffered bulk insert operation, updates the non-persistent table stats. row_merge_bulk_t::write_to_index(): Update stat_n_rows after applying the bulk insert operation row_ins_clust_index_entry_low(): In case of copy algorithm, switch to bulk insert operation. copy_data_error_ignore(): Handles the error while copying the data from source to target file.
Thirunarayanan
force-pushed
the
10.11-MDEV-33087
branch
from
1f66c6c
to
55fb366
Compare
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.
Description
How can this PR be tested?
Tested with large data set:
Existing alter copy algorithm:
MariaDB [test]> alter table t1 force, algorithm=copy;
Query OK, 6291456 rows affected (12 min 18.519 sec)
Records: 6291456 Duplicates: 0 Warnings: 0
With bulk insert (copy algorithm)
MariaDB [test]> alter table t1 force, algorithm=copy;
Query OK, 6291456 rows affected (7 min 30.760 sec)
Records: 6291456 Duplicates: 0 Warnings: 0
create..select copy algorithm:
MariaDB [test]> create table t2 select * from t1;
Query OK, 6619136 rows affected (7 min 5.126 sec)
Records: 6619136 Duplicates: 0 Warnings: 0
create..select (with bulk insert):
MariaDB [test]> create table t2 select * from t1;
Query OK, 6619136 rows affected (4 min 54.647 sec)
Records: 6619136 Duplicates: 0 Warnings: 0
Basing the PR against the correct MariaDB version
PR quality check