Optimize MyRocks Replace Into Statement #913

justfortaste · 2018-12-13T12:32:50Z

Summary:
When rocksdb tables only have primary key and without secondary indexes,
Replace Into can use Insert Into instead and this insert into no
need to check unique constraint.

facebook-github-bot

@yoshinorim has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

yoshinorim · 2018-12-13T14:06:55Z

mysql-test/suite/rocksdb_rpl/r/optimize_myrocks_replace_info.result

Outputs of "show binlog events" may change between versions or build types, so they make tests unstable. Instead, how about checking Get count from perf context for each replace, in both master and slave? Expected behavior is Get count does not increase in both master and slave.

select value into @g from information_schema.rocksdb_perf_context_global where stat_type='GET_FROM_MEMTABLE_COUNT'; REPLACE... select case when value-@c > 0 then 'false' else 'true' end as read_free from information_schema.rocksdb_perf_context_global where stat_type='GET_FROM_MEMTABLE_COUNT';

rocksdb_perf_context_level should be non zero, to count Gets (3 is fine).

That is a good idead,
use -- source include/show_binlog_events.inc is also ok.

You may combine both rocksdb_perf_context_global and source include/show_binlog_events.inc. Raw "show binlog events" caused test failures on some of our builds, since version strings were different.

@@ -21,7 +21,7 @@ replace into t1 values(2,44),(3,55); show binlog events; Log_name Pos Event_type Server_id End_log_pos Info -master-bin.000001 4 Format_desc 1 120 Server ver: 5.6.35-log, Binlog ver: 4 +master-bin.000001 4 Format_desc 1 120 Server ver: 5.6.35-fb-log, Binlog ver: 4

yoshinorim · 2018-12-13T14:09:45Z

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_info.test

Could you add another test case about locking -- 1. "replace into t1 values(1,11);" blocks updates/deletes to the same key (1) until a transaction is committed, and 2. replace is blocked if there is already a running transaction locking the same key?

yoshinorim

Thanks a lot for the awesome feature! Overall code looks good to me. I am asking a few more people to review. I have a few requests about your test case.

facebook-github-bot

@yns88 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: When rocksdb tables only have primary key and without secondary indexes, Replace Into can use Insert Into instead and this insert into no need to check unique constraint.

facebook-github-bot · 2018-12-14T03:09:17Z

@justfortaste has updated the pull request. Re-import the pull request

facebook-github-bot

@yoshinorim has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_into.test

mysql-test/suite/rocksdb_rpl/r/optimize_myrocks_replace_into.result

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_into.test

mysql-test/suite/rocksdb/t/optimize_myrocks_replace_into_lock.test

laurynas-biveinis · 2018-12-14T06:19:54Z

mysql-test/suite/rocksdb/t/optimize_myrocks_replace_into_lock.test

+
+create table t1(c1 int,c2 int, primary key (c1)) engine=rocksdb;
+insert into t1 values(1,1),(2,2),(3,3);
+select * from t1;


IMHO redundant

laurynas-biveinis · 2018-12-14T06:20:49Z

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_into.test

@@ -0,0 +1,38 @@
+--source include/have_rocksdb.inc
+--source include/master-slave.inc
+--source include/have_binlog_format_row.inc


Please swap these two: the master-slave.inc is heavy, it sets up replication, and have_binlog_format_row.inc will throw away all that work for the other two replication formats

Replace into bline write only effect row base replication, Maybe I need to make a new testcase to test all replication formats

laurynas-biveinis · 2018-12-14T06:21:01Z

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_into.test

+--source include/master-slave.inc
+--source include/have_binlog_format_row.inc
+
+connection master;


laurynas-biveinis · 2018-12-14T06:21:16Z

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_into.test

+
+create table t1(c1 int,c2 int, primary key (c1)) engine=rocksdb;
+insert into t1 values(1,1),(2,2),(3,3);
+select * from t1;


IMHO redundant

laurynas-biveinis · 2018-12-14T06:21:45Z

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_into.test

+select case when value-@c > 0 then 'false' else 'true' end as read_free from information_schema.rocksdb_perf_context where table_schema='test' and table_name='t1' and stat_type='GET_FROM_MEMTABLE_COUNT';
+--source include/show_binlog_events.inc
+
+sync_slave_with_master;


Consider --source include/sync_slave_sql_with_master.inc instead

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_into.test

laurynas-biveinis · 2018-12-14T06:25:03Z

storage/rocksdb/ha_rocksdb.cc

  DBUG_ASSERT(found != nullptr);
  DBUG_ASSERT(pk_changed != nullptr);

+  bool ignore_get = false;


Declare at initialization instead?

facebook-github-bot · 2018-12-14T06:26:00Z

@justfortaste has updated the pull request. Re-import the pull request

facebook-github-bot · 2018-12-14T07:12:40Z

@justfortaste has updated the pull request. Re-import the pull request

facebook-github-bot

@yoshinorim has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

hermanlee

Can a variable setting controlling the use of the replace into optimization and binlog modifications be added?

hermanlee · 2018-12-21T18:21:33Z

sql/log_event.h

+/**
+  flag for myrocks replace into
+ */
+#define LOG_EVENT_DUP_REPLACE_F 0x400


It seems like this could easily collide with changes made by Oracle, even though it looks like 8.0 hasn't added anything new here. It also seems this might be better off in the row_log_event structure rather than use one up one of flag bits that apply to all binlog events. There are also 16 bits there and only 4 are used so far. Using one of the upper bits there would have a small chance of collision.

hermanlee · 2018-12-21T18:39:15Z

storage/rocksdb/ha_rocksdb.cc


+  ignore_get = can_optimize_replace_into();
+  if (ignore_get)
+    ha_thd()->lex->can_optimize_replace_into = true;


This seems to be the wrong dependency ordering to me. Here, MyRocks determines if optimized replace_into can be used, if so, then can_optimize_replace_into is set, which then tells the binlog to write out the LOG_EVENT_DUP_REPLACE_F bit. So binlog's decision to use DUP_REPLACE depends on MyRocks, which depends on MySQL's processing of the command. But binlog's decision to indicate DUP_REPLACE should be independent of the storage engine.

Would a MyRocks slave reading binlog entries from an InnoDB master which processed the REPLACE query be allowed to use the optimization if it satisfies the other conditions (no hidden primary key, no triggers).

Alternatively, if a MyRocks master had triggers set on its table, but a MyRocks slave did not have triggers, should the MyRocks slave be able to perform the REPLACE optimization too?

hermanlee · 2018-12-21T18:47:45Z

mysql-test/suite/rocksdb/t/optimize_myrocks_replace_into_base.test

+SET GLOBAL rocksdb_perf_context_level=3;
+
+#
+# cast 1: table only with primary key, support replace blind write


These test cases are great. Can they be extended to master/slave setups too to verify the slaves? But we'd also want to test combinations where the master has triggers/secondary keys, the slave doesn't, or where the slave has triggers/secondary keys, but the master does not.

The reason these cases are important is because schema changes and triggers are applied to each member of a replicaset at different times, so there are times when schemas between master and slave will different.

Summary: This is a diff based of #913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Reviewed By: abhinav04sharma Differential Revision: D14304600 fbshipit-source-id: f14c64c

Summary: This is a diff based of facebook#913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Reviewed By: abhinav04sharma Differential Revision: D14304600 fbshipit-source-id: f14c64c

Summary: This is a diff based of #913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Reviewed By: abhinav04sharma Differential Revision: D14304600 fbshipit-source-id: be7cfcb

Summary: This is a diff based of #913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Reference Patch: f14c64cf950 Pull Request resolved: #1124 Reviewed By: yizhang82 Differential Revision: D21845477 fbshipit-source-id: 0617d534b47

Summary: This is a diff based of facebook#913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Reviewed By: abhinav04sharma Differential Revision: D14304600 fbshipit-source-id: be7cfcb

Summary: This is a diff based of facebook#913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Reference Patch: facebook@f14c64cf950 Pull Request resolved: facebook#1124 Reviewed By: yizhang82 Differential Revision: D21845477 fbshipit-source-id: 0617d534b47

Summary: This is a diff based of facebook#913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Differential Revision: D14304600

) Summary: This is a diff based of facebook#913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Reference Patch: facebook@f14c64cf950 Pull Request resolved: facebook#1124 Differential Revision: D21845477

Summary: This is a diff based of facebook/mysql-5.6#913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Differential Revision: D14304600

) Summary: This is a diff based of facebook#913. The initial pull request was submitted by Zhang Yuan from Alibaba. I am trying to continue work on this. Approach: During sql_insert, if the statement is a 'replace into' statement, then we set a boolean in the lex to indicate that primary key violations can be ignored. This is done only for tables that have an explicitly defined primary key, does not have any triggers or secondary keys. If the lex flag indicates that this is a blind replace into, then engine ignores duplicate primary key errors. This in turn ensures that the sql layer does not have to do a delete followed by a insert. If the lex flag is set, then Write_rows_log_event sets a bit in its flag indicating that this was an optimized 'replace into' statement execution. This is carried over to the slave and the slave sets the lex flag (in Write_rows_log_event::write_row()) before invoking the handler's write_row methods. On the slave, if blind replace bit is set for a write rows event and it is possible to do a blind replace, we will proceed with it. If for some reason (such as slaves having triggers or secondary keys), we cannot do a blind replace then this will fall back to doing either an update or a delete+insert. Note that the binlog will not have a before image (since on master this is just an insert), but we can still do the regular update or delete+insert. Reference Patch: facebook@f14c64cf950 Pull Request resolved: facebook#1124 Differential Revision: D21845477

facebook-github-bot added the CLA Signed label Dec 13, 2018

facebook-github-bot reviewed Dec 13, 2018

View reviewed changes

yoshinorim reviewed Dec 13, 2018

View reviewed changes

justfortaste changed the title ~~Optimze MyRocks Replace Into Statement~~ Dec 13, 2018

facebook-github-bot reviewed Dec 13, 2018

View reviewed changes

Optimize MyRocks Replace Into Statement

e1429c8

Summary: When rocksdb tables only have primary key and without secondary indexes, Replace Into can use Insert Into instead and this insert into no need to check unique constraint.

justfortaste force-pushed the feature_optimize_myrocks_replace_into branch from 9519f1e to e1429c8 Compare December 14, 2018 03:09

facebook-github-bot reviewed Dec 14, 2018

View reviewed changes

yoshinorim reviewed Dec 14, 2018

View reviewed changes

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_into.test Show resolved Hide resolved

yoshinorim reviewed Dec 14, 2018

View reviewed changes

mysql-test/suite/rocksdb_rpl/r/optimize_myrocks_replace_into.result Show resolved Hide resolved

yoshinorim reviewed Dec 14, 2018

View reviewed changes

mysql-test/suite/rocksdb_rpl/t/optimize_myrocks_replace_into.test Show resolved Hide resolved

laurynas-biveinis reviewed Dec 14, 2018

View reviewed changes

Modify testcase for myrocks replace into optimize

4fd393d

justfortaste force-pushed the feature_optimize_myrocks_replace_into branch from cdd7bf9 to 4fd393d Compare December 14, 2018 07:12

facebook-github-bot reviewed Dec 15, 2018

View reviewed changes

hermanlee reviewed Dec 21, 2018

View reviewed changes

oleksandr-kachan mentioned this pull request Apr 17, 2020

Optimize replace into to do a blind write #1124

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize MyRocks Replace Into Statement #913

Optimize MyRocks Replace Into Statement #913

Uh oh!

justfortaste commented Dec 13, 2018

facebook-github-bot left a comment

yoshinorim Dec 13, 2018 •

edited

Loading

justfortaste Dec 13, 2018

yoshinorim Dec 14, 2018

yoshinorim Dec 13, 2018

justfortaste Dec 13, 2018

yoshinorim left a comment

facebook-github-bot left a comment

facebook-github-bot commented Dec 14, 2018

facebook-github-bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

laurynas-biveinis Dec 14, 2018

laurynas-biveinis Dec 14, 2018

justfortaste Dec 14, 2018

laurynas-biveinis Dec 14, 2018

laurynas-biveinis Dec 14, 2018

laurynas-biveinis Dec 14, 2018

Uh oh!

laurynas-biveinis Dec 14, 2018

facebook-github-bot commented Dec 14, 2018

facebook-github-bot commented Dec 14, 2018

facebook-github-bot left a comment

hermanlee left a comment

hermanlee Dec 21, 2018

hermanlee Dec 21, 2018

hermanlee Dec 21, 2018

Labels

5 participants

Optimize MyRocks Replace Into Statement #913

Are you sure you want to change the base?

Optimize MyRocks Replace Into Statement #913

Uh oh!

Conversation

justfortaste commented Dec 13, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

yoshinorim Dec 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoshinorim left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Dec 14, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

facebook-github-bot commented Dec 14, 2018

facebook-github-bot commented Dec 14, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

hermanlee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Labels

5 participants

yoshinorim Dec 13, 2018 •

edited

Loading