Test against MySQL 8.0 (#150)

Shopify · Sep 20, 2023 · cb3a882 · cb3a882
1 parent f6070c1
commit cb3a882
Show file tree

Hide file tree

Showing 11 changed files with 146 additions and 27 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -17,12 +17,16 @@ jobs:
         include:
           - ruby-version: "3.0"
             activerecord-version: "6.0"
+            mysql_version: "5.7"
           - ruby-version: "3.1"
             activerecord-version: "6.1"
+            mysql_version: "5.7"
           - ruby-version: "3.2"
             activerecord-version: "7.0"
+            mysql_version: "8.0"
           - ruby-version: "3.2"
             activerecord-version: "7.1.0.beta1"
+            mysql_version: "8.0"
     steps:
       - uses: actions/checkout@v2
       - name: Set up Ruby
@@ -35,7 +39,7 @@ jobs:
       - name: Install Ubuntu packages
         run: sudo apt-get update && sudo apt-get install numactl libaio-dev libmysqlclient-dev
       - name: Setup MySQL and ProxySQL (docker-compose)
-        run: docker-compose up -d # Might have to change to docker compose up -d (i.e. Compose V2) when the Ubuntu image changes the docker-compose version
+        run: docker-compose -f docker-compose-mysql-${{ matrix.mysql_version }}.yml up -d # Might have to change to docker compose up -d (i.e. Compose V2) when the Ubuntu image changes the docker-compose version
       - name: Wait until DBs are alive
         run: ./scripts/helpers/wait-for-dbs.sh
         timeout-minutes: 2

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,5 @@
 # Unreleased
+* Test against MySQL 8.0.
 
 # 4.0.0 (Sep, 2023)
 * Deprecate `SlaveLag` throttler class name. Use `ReplicaLag` instead (https://github.com/Shopify/lhm/pull/144)

diff --git a/dev.yml b/dev.yml
@@ -41,6 +41,9 @@ commands:
     run: podman-compose logs -f
   clear:
     run: podman-compose down -v && podman-compose up -d && ./scripts/helpers/wait-for-dbs.sh
+    subcommands:
+      mysql-5.7: podman-compose down -v && podman-compose -f docker-compose-mysql-5.7.yml up -d && ./scripts/helpers/wait-for-dbs.sh
+      mysql-8.0: podman-compose down -v && podman-compose -f docker-compose-mysql-8.0.yml up -d && ./scripts/helpers/wait-for-dbs.sh
   pre-publish:
     # Ensures all Gemfile.lock are sync with the new version in `lhm/version.rb` and runs appraisals
     run: bundle install && bundle exec appraisal install && bundle exec appraisal rake specs
diff --git a/docker-compose-mysql-5.7.yml b/docker-compose-mysql-5.7.yml
@@ -0,0 +1 @@
+docker-compose.yml
diff --git a/docker-compose-mysql-8.0.yml b/docker-compose-mysql-8.0.yml
@@ -0,0 +1,63 @@
+services:
+  #  Writer
+  mysql-1:
+    container_name: mysql-1
+    image: percona:8.0
+    platform: linux/amd64
+    command:
+      --server-id=1
+      --log-bin
+      --log-slave-updates=ON
+      --gtid-mode=ON
+      --enforce-gtid-consistency=ON
+      --read-only=OFF
+      --max-connections=1000
+      --default-authentication-plugin=mysql_native_password
+    hostname: 'mysql-1'
+    volumes:
+      - ./scripts/mysql/writer:/docker-entrypoint-initdb.d
+    environment:
+      MYSQL_ROOT_PASSWORD: password
+      MYSQL_HOST: mysql-1
+    ports:
+      - "33006:3306"
+  #  Reader
+  mysql-2:
+    container_name: mysql-2
+    image: percona:8.0
+    platform: linux/amd64
+    command:
+      --server-id=2
+      --log-bin
+      --log-slave-updates=ON
+      --gtid-mode=ON
+      --enforce-gtid-consistency=ON
+      --read-only=ON
+      --max-connections=1000
+      --default-authentication-plugin=mysql_native_password
+    hostname: 'mysql-2'
+    volumes:
+      - ./scripts/mysql/reader:/docker-entrypoint-initdb.d
+    environment:
+      MYSQL_ROOT_PASSWORD: password
+      MYSQL_HOST: mysql-2
+    ports:
+      - "33007:3306"
+  #  Proxysql
+  proxysql:
+    container_name: proxysql
+    image: proxysql/proxysql:2.0.11
+    platform: linux/amd64
+    volumes:
+      - ./scripts/proxysql/proxysql.cnf:/etc/proxysql.cnf
+    command: "proxysql -c /etc/proxysql.cnf -f --idle-threads"
+    ports:
+      - "33005:3306"
+      - "6032:6032"
+  toxiproxy:
+    container_name: toxiproxy
+    image: "ghcr.io/shopify/toxiproxy"
+    ports:
+      - "8474:8474"
+      - "22220:22220"
+      - "22222:22222"
diff --git a/scripts/mysql/writer/create_users.sql b/scripts/mysql/writer/create_users.sql
@@ -3,4 +3,4 @@ CREATE USER IF NOT EXISTS 'writer'@'%' IDENTIFIED BY 'password';
 CREATE USER IF NOT EXISTS 'reader'@'%' IDENTIFIED BY 'password';
 
 CREATE USER IF NOT EXISTS 'replication'@'%' IDENTIFIED BY 'password';
-GRANT REPLICATION SLAVE ON *.* TO' replication'@'%' IDENTIFIED BY 'password';
+GRANT REPLICATION SLAVE ON *.* TO' replication'@'%';
diff --git a/spec/integration/chunker_spec.rb b/spec/integration/chunker_spec.rb
@@ -74,7 +74,8 @@ def log_messages
         Lhm::Chunker.new(migration, connection, {raise_on_warnings: true, throttler: throttler, printer: printer} ).run
       end
 
-      assert_match "Unexpected warning found for inserted row: Duplicate entry '1001' for key 'index_custom_primary_key_on_id'", exception.message
+      error_key = index_key("custom_primary_key_dest", "index_custom_primary_key_on_id")
+      assert_match "Unexpected warning found for inserted row: Duplicate entry '1001' for key '#{error_key}'", exception.message
     end
 
     it 'should copy and warn on unexpected warnings by default' do
@@ -87,8 +88,10 @@ def log_messages
 
       Lhm::Chunker.new(migration, connection, {throttler: throttler, printer: printer} ).run
 
+      error_key = index_key("custom_primary_key_dest", "index_custom_primary_key_on_id")
+
       assert_equal 2, log_messages.length
-      assert log_messages[1].include?("Unexpected warning found for inserted row: Duplicate entry '1001' for key 'index_custom_primary_key_on_id'"), log_messages
+      assert log_messages[1].include?("Unexpected warning found for inserted row: Duplicate entry '1001' for key '#{error_key}'"), log_messages
     end
 
     it 'should log two times for two unexpected warnings' do
@@ -103,9 +106,11 @@ def log_messages
 
       Lhm::Chunker.new(migration, connection, {throttler: throttler, printer: printer} ).run
 
+      error_key = index_key("custom_primary_key_dest", "index_custom_primary_key_on_id")
+
       assert_equal 3, log_messages.length
-      assert log_messages[1].include?("Unexpected warning found for inserted row: Duplicate entry '1001' for key 'index_custom_primary_key_on_id'"), log_messages
-      assert log_messages[2].include?("Unexpected warning found for inserted row: Duplicate entry '1002' for key 'index_custom_primary_key_on_id'"), log_messages
+      assert log_messages[1].include?("Unexpected warning found for inserted row: Duplicate entry '1001' for key '#{error_key}'"), log_messages
+      assert log_messages[2].include?("Unexpected warning found for inserted row: Duplicate entry '1002' for key '#{error_key}'"), log_messages
     end
 
     it 'should copy and warn on unexpected warnings' do
@@ -118,8 +123,10 @@ def log_messages
 
       Lhm::Chunker.new(migration, connection, {raise_on_warnings: false, throttler: throttler, printer: printer} ).run
 
+      error_key = index_key("custom_primary_key_dest", "index_custom_primary_key_on_id")
+
       assert_equal 2, log_messages.length
-      assert log_messages[1].include?("Unexpected warning found for inserted row: Duplicate entry '1001' for key 'index_custom_primary_key_on_id'"), log_messages
+      assert log_messages[1].include?("Unexpected warning found for inserted row: Duplicate entry '1001' for key '#{error_key}'"), log_messages
     end
 
     it 'should create the modified destination, even if the source is empty' do
@@ -261,4 +268,12 @@ def throttler.replica_connection(replica)
       end
     end
   end
+
+  def index_key(table_name, index_name)
+    if mysql_version.start_with?("8")
+      "#{table_name}.#{index_name}"
+    else
+      index_name
+    end
+  end
 end
diff --git a/spec/integration/integration_helper.rb b/spec/integration/integration_helper.rb
@@ -35,6 +35,14 @@ def connection
     @connection
   end
 
+  def mysql_version
+    @mysql_version ||= begin
+      # This SQL returns a value of shape: X.Y.ZZ-AA-log
+      result = connection.query("SELECT VERSION()")
+      result.dig(0, 0).split("-", 2)[0]
+    end
+  end
+
   def connect_proxysql!
     connect!(
       '127.0.0.1',

diff --git a/spec/integration/lhm_spec.rb b/spec/integration/lhm_spec.rb
@@ -9,6 +9,10 @@
 
   before(:each) { connect_master!; Lhm.cleanup(true) }
 
+  let(:collation) do
+    mysql_version.start_with?("8.0") ? "utf8mb3_general_ci" : "utf8_general_ci"
+  end
+
   describe 'id column requirement' do
     it 'should migrate the table when id is pk' do
       table_create(:users)
@@ -17,9 +21,11 @@
         t.add_column(:logins, "int(12) default '0'")
       end
 
+      expected_type = mysql_version.start_with?("8.0") ? "int" : "int(12)"
+
       replica do
         value(table_read(:users).columns['logins']).must_equal({
-          :type           => 'int(12)',
+          :type           => expected_type,
           :is_nullable    => 'YES',
           :column_default => '0',
           :comment => '',
@@ -35,9 +41,11 @@
         t.add_column(:logins, "int(12) default '0'")
       end
 
+      expected_type = mysql_version.start_with?("8.0") ? "int" : "int(12)"
+
       replica do
         value(table_read(:custom_primary_key).columns['logins']).must_equal({
-          :type           => 'int(12)',
+          :type           => expected_type,
           :is_nullable    => 'YES',
           :column_default => '0',
           :comment => '',
@@ -53,9 +61,11 @@
         t.add_column(:logins, "int(12) default '0'")
       end
 
+      expected_type = mysql_version.start_with?("8.0") ? "int" : "int(12)"
+
       replica do
         value(table_read(:composite_primary_key).columns['logins']).must_equal({
-          :type           => 'int(12)',
+          :type           => expected_type,
           :is_nullable    => 'YES',
           :column_default => '0',
           :comment => '',
@@ -132,9 +142,11 @@
         t.add_column(:logins, "INT(12) DEFAULT '0'")
       end
 
+      expected_type = mysql_version.start_with?("8.0") ? "int" : "int(12)"
+
       replica do
         value(table_read(:users).columns['logins']).must_equal({
-          :type => 'int(12)',
+          :type => expected_type,
           :is_nullable => 'YES',
           :column_default => '0',
           :comment => '',
@@ -272,7 +284,7 @@
           :is_nullable => 'NO',
           :column_default => 'none',
           :comment => '',
-          :collate => 'utf8_general_ci',
+          :collate => collation,
         })
       end
     end
@@ -284,9 +296,11 @@
         t.change_column(:id, 'int(5)')
       end
 
+      expected_type = mysql_version.start_with?("8.0") ? "int" : "int(5)"
+
       replica do
         value(table_read(:small_table).columns['id']).must_equal({
-          :type => 'int(5)',
+          :type => expected_type,
           :is_nullable => 'NO',
           :column_default => nil,
           :comment => '',
@@ -311,7 +325,7 @@
           :is_nullable => 'YES',
           :column_default => nil,
           :comment => '',
-          :collate => 'utf8_general_ci',
+          :collate => collation,
         })
 
         result = select_one('SELECT login from users')
@@ -336,7 +350,7 @@
           :is_nullable => 'YES',
           :column_default => 'Superfriends',
           :comment => '',
-          :collate => 'utf8_general_ci',
+          :collate => collation,
         })
 
         result = select_one('SELECT `fnord` from users')
@@ -383,11 +397,13 @@
         t.rename_column(:reference, :ref)
       end
 
+      expected_type = mysql_version.start_with?("8.0") ? "int" : "int(11)"
+
       replica do
         table_data = table_read(:users)
         assert_nil table_data.columns['reference']
         value(table_read(:users).columns['ref']).must_equal({
-           :type => 'int(11)',
+           :type => expected_type,
            :is_nullable => 'YES',
            :column_default => nil,
            :comment => 'RefComment',
@@ -418,7 +434,7 @@
            :is_nullable => 'YES',
            :column_default => nil,
            :comment => '',
-           :collate => 'utf8_general_ci',
+           :collate => collation,
          })
 
         result = select_one('SELECT `fnord` from users')
@@ -443,7 +459,7 @@
           :is_nullable => 'YES',
           :column_default => nil,
           :comment => '',
-          :collate => 'utf8_general_ci',
+          :collate => collation,
         })
 
         result = select_one('SELECT `user_name` from users')
@@ -470,7 +486,7 @@
           :is_nullable => 'NO',
           :column_default => nil,
           :comment => '',
-          :collate => 'utf8_general_ci',
+          :collate => collation,
         })
 
         result = select_one('SELECT `user_name` from users')
@@ -517,7 +533,7 @@
           :is_nullable => 'YES',
           :column_default => 'Superfriends',
           :comment => '',
-          :collate => 'utf8_general_ci',
+          :collate => collation,
         })
       end
     end

diff --git a/spec/integration/sql_retry/lock_wait_timeout_test_helper.rb b/spec/integration/sql_retry/lock_wait_timeout_test_helper.rb
@@ -14,13 +14,21 @@ def initialize(lock_duration:, innodb_lock_wait_timeout:)
 
     @lock_duration = lock_duration
 
-    # While implementing this, I discovered that MySQL seems to have an off-by-one
-    # bug with the innodb_lock_wait_timeout. If you ask it to wait 2 seconds, it will wait 3.
-    # In order to avoid surprisingly the user, let's account for that here, but also
-    # guard against a case where we go below 1, the minimum value.
-    raise ArgumentError, "innodb_lock_wait_timeout must be greater than or equal to 2" unless innodb_lock_wait_timeout >= 2
     raise ArgumentError, "innodb_lock_wait_timeout must be an integer" if innodb_lock_wait_timeout.class != Integer
-    @innodb_lock_wait_timeout = innodb_lock_wait_timeout - 1
+
+    result = @main_conn.query("SELECT VERSION()")
+    mysql_version = result.to_a.dig(0, "VERSION()").split("-", 2)[0]
+
+    if mysql_version.start_with?("8")
+      @innodb_lock_wait_timeout = innodb_lock_wait_timeout
+    else
+      # While implementing this, I discovered that MySQL seems to have an off-by-one
+      # bug with the innodb_lock_wait_timeout. If you ask it to wait 2 seconds, it will wait 3.
+      # In order to avoid surprisingly the user, let's account for that here, but also
+      # guard against a case where we go below 1, the minimum value.
+      raise ArgumentError, "innodb_lock_wait_timeout must be greater than or equal to 2" unless innodb_lock_wait_timeout >= 2
+      @innodb_lock_wait_timeout = innodb_lock_wait_timeout - 1
+    end
 
     @threads = []
     @queue = Queue.new

diff --git a/spec/integration/table_spec.rb b/spec/integration/table_spec.rb
@@ -23,7 +23,7 @@
       end
 
       it 'should parse columns' do
-        value(@table.columns['id'][:type]).must_match(/(bigint|int)\(\d+\)/)
+        value(@table.columns['id'][:type]).must_match(/(bigint|int)(\(\d+\))?/)
       end
 
       it 'should return true for method that should be renamed' do