From ff0b6735a232ee2503fb8cc1382b8c8c50cfd288 Mon Sep 17 00:00:00 2001 From: Matthew Riddle Date: Fri, 27 Mar 2020 17:07:01 +0100 Subject: [PATCH] Remove ServerVariables module This module adds confusion to the implementation, one could mistakenly think it's how to configure a production database, instead, remove it and rely on the database.yml to have the variables set on in the dev and test environment for this gem. For production, clients should be setting the values directly in their MySQL configuration. https://dev.mysql.com/doc/refman/5.6/en/replication-options-master.html#sysvar_auto_increment_increment The `offset` variables were also unused and have been removed --- lib/global_uid.rb | 2 -- lib/global_uid/base.rb | 8 ++------ lib/global_uid/server_variables.rb | 30 ------------------------------ test/config/database.yml | 6 ++++++ test/global_uid_test.rb | 2 +- test/test_helper.rb | 2 -- 6 files changed, 9 insertions(+), 41 deletions(-) delete mode 100644 lib/global_uid/server_variables.rb diff --git a/lib/global_uid.rb b/lib/global_uid.rb index bbc8c42..ab368ce 100644 --- a/lib/global_uid.rb +++ b/lib/global_uid.rb @@ -12,8 +12,6 @@ class ConnectionTimeoutException < StandardError ; end class TimeoutException < StandardError ; end class InvalidIncrementException < StandardError ; end class InvalidIncrementPreventionException < StandardError ; end - - autoload :ServerVariables, "global_uid/server_variables" end ActiveRecord::Base.send(:include, GlobalUid::ActiveRecordExtension) diff --git a/lib/global_uid/base.rb b/lib/global_uid/base.rb index 6574ed0..16ed299 100644 --- a/lib/global_uid/base.rb +++ b/lib/global_uid/base.rb @@ -50,7 +50,7 @@ def self.drop_uid_tables(id_table_name, options={}) end end - def self.new_connection(name, connection_timeout, offset, increment_by) + def self.new_connection(name, connection_timeout) raise "No id server '#{name}' configured in database.yml" unless ActiveRecord::Base.configurations.to_h.has_key?(name) config = ActiveRecord::Base.configurations.to_h[name] c = config.symbolize_keys @@ -76,18 +76,14 @@ def self.init_server_info(options) raise "You haven't configured any id servers" if id_servers.nil? or id_servers.empty? raise "More servers configured than increment_by: #{id_servers.size} > #{options[:increment_by]} -- this will create duplicate IDs." if id_servers.size > options[:increment_by] - offset = 1 - id_servers.map do |name, i| info = {} info[:cx] = nil info[:name] = name info[:retry_at] = nil - info[:offset] = offset info[:rand] = rand info[:new?] = true info[:allocated_ids] = Allocations.new(incrementing_by: self.global_uid_increment_by) - offset +=1 info end end @@ -113,7 +109,7 @@ def self.setup_connections!(options) info[:new?] = false begin - connection = new_connection(info[:name], connection_timeout, info[:offset], increment_by) + connection = new_connection(info[:name], connection_timeout) if !connection.nil? && increment_by != (db_increment = connection.select_value("SELECT @@auto_increment_increment")) raise InvalidIncrementPreventionException, "Configured: '#{increment_by}', Found: '#{db_increment}' on '#{connection.current_database}'" end diff --git a/lib/global_uid/server_variables.rb b/lib/global_uid/server_variables.rb deleted file mode 100644 index 09069d7..0000000 --- a/lib/global_uid/server_variables.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true -# This module is good for testing and development, not so much for production. -# Please note that this is unreliable -- if you lose your CX to the server -# and auto-reconnect, you will be utterly hosed. Much better to dedicate a server -# or two to the cause, and set their auto_increment_increment globally. -# -# You can include this module in tests like this: -# GlobalUid::Base.extend(GlobalUid::ServerVariables) -# -module GlobalUid - module ServerVariables - - def self.extended(base) - base.singleton_class.send(:alias_method, :new_connection_without_server_variables, :new_connection) - base.singleton_class.send(:alias_method, :new_connection, :new_connection_with_server_variables) - end - - def new_connection_with_server_variables(name, connection_timeout, offset, increment_by) - con = new_connection_without_server_variables(name, connection_timeout, offset, increment_by) - - if con - con.execute("set @@auto_increment_increment = #{increment_by}") - con.execute("set @@auto_increment_offset = #{offset}") - end - - con - end - - end -end diff --git a/test/config/database.yml b/test/config/database.yml index 09eac8b..f9af9d7 100644 --- a/test/config/database.yml +++ b/test/config/database.yml @@ -15,7 +15,13 @@ test: test_id_server_1: <<: *MYSQL database: global_uid_test_id_server_1 + variables: + auto_increment_increment: 5 + auto_increment_offset: 1 test_id_server_2: <<: *MYSQL database: global_uid_test_id_server_2 + variables: + auto_increment_increment: 5 + auto_increment_offset: 2 diff --git a/test/global_uid_test.rb b/test/global_uid_test.rb index cdbb44d..1808271 100644 --- a/test/global_uid_test.rb +++ b/test/global_uid_test.rb @@ -338,7 +338,7 @@ def table_exists?(connection, table) describe "With a timing out server" do before do reset_connections! - @a_decent_cx = GlobalUid::Base.new_connection(GlobalUid::Base.global_uid_servers.first, 50, 1, 5) + @a_decent_cx = GlobalUid::Base.new_connection(GlobalUid::Base.global_uid_servers.first, 50) ActiveRecord::Base.stubs(:mysql2_connection).raises(GlobalUid::ConnectionTimeoutException).then.returns(@a_decent_cx) @connections = GlobalUid::Base.get_connections end diff --git a/test/test_helper.rb b/test/test_helper.rb index 57864f6..d33c461 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -8,8 +8,6 @@ require 'global_uid' require 'phenix' -GlobalUid::Base.extend(GlobalUid::ServerVariables) - Phenix.configure do |config| config.database_config_path = File.join(File.dirname(__FILE__), "config/database.yml") end