Merge pull request #69 from zendesk/mriddle-refactor

Add a performance spec

.circleci/config.yml

@@ -6,6 +6,7 @@ run_tests: &run_tests
     - run: sudo apt install -y default-mysql-client
     - run: bundle install
     - run: bundle exec rake test
+    - run: bundle exec rake performance_test
 
 jobs:
 

Rakefile

@@ -3,9 +3,14 @@ require 'bundler/gem_tasks'
 require 'bump/tasks'
 require 'rake/testtask'
 
-task :default => 'test'
+task :default => ['test', 'performance_test']
 
 Rake::TestTask.new(:test) do |test|
-  test.pattern = 'test/**/*_test.rb'
+  test.pattern = 'test/lib/*_test.rb'
+  test.verbose = true
+end
+
+Rake::TestTask.new(:performance_test) do |test|
+  test.pattern = 'test/performance/*_test.rb'
   test.verbose = true
 end

global_uid.gemspec

@@ -18,6 +18,7 @@ Gem::Specification.new 'global_uid', '3.7.1' do |s|
   s.add_development_dependency('minitest-rg')
   s.add_development_dependency('minitest-line')
   s.add_development_dependency('mocha')
+  s.add_development_dependency('benchmark-ips')
   s.add_development_dependency('bump')
   s.add_development_dependency('phenix')
   s.add_development_dependency('pry')

lib/global_uid/base.rb

@@ -9,7 +9,7 @@ class Base
     GLOBAL_UID_DEFAULTS = {
       :connection_timeout   => 3,
       :connection_retry     => 10.minutes,
-      :notifier             => Proc.new { |exception, message| ActiveRecord::Base.logger.error("GlobalUID error:  #{exception} #{message}") },
+      :notifier             => Proc.new { |exception, message| ActiveRecord::Base.logger.error("GlobalUID error: #{exception.class} #{message}") },
       :query_timeout        => 10,
       :increment_by         => 5,  # This will define the maximum number of servers that you can have
       :disabled             => false,

test/global_uid_test.rb → test/lib/global_uid_test.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
-require_relative 'test_helper'
-require_relative 'migrations'
-require_relative 'models'
+require_relative '../test_helper'
 
 describe GlobalUid do
   before do
@@ -35,6 +33,17 @@
       assert !ParentSubclass.global_uid_disabled
       assert !ParentSubclassSubclass.global_uid_disabled
     end
+
+    it "uses the default AUTO_INCREMENT, skipping the alloc servers" do
+      CreateWithoutGlobalUIDs.up
+      GlobalUid::Base.expects(:get_uid_for_class).never
+
+      (1..10).each do |index|
+        assert_equal index, WithoutGlobalUID.create!.id
+      end
+
+      CreateWithoutGlobalUIDs.down
+    end
   end
 
   describe "migrations" do
@@ -394,27 +403,6 @@ def parent_child_fork_values
 
   private
 
-  def test_unique_ids
-    seen = {}
-    (0..10).each do
-      foo = WithGlobalUID.new
-      foo.save
-      refute_nil foo.id
-      assert_nil foo.description
-      refute seen.has_key?(foo.id)
-      seen[foo.id] = 1
-    end
-  end
-
-  def reset_connections!
-    GlobalUid::Base.servers = nil
-  end
-
-  def restore_defaults!
-    GlobalUid::Base.global_uid_options[:storage_engine] = nil
-    GlobalUid::Base.global_uid_options[:disabled] = false
-  end
-
   def show_create_sql(klass, table)
     klass.connection.select_rows("show create table #{table}")[0][1]
   end

test/performance/creation_test.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+require_relative '../test_helper'
+
+describe GlobalUid do
+  before do
+    Phenix.rise!(with_schema: false)
+    ActiveRecord::Base.establish_connection(:test)
+    reset_connections!
+    restore_defaults!
+  end
+
+  after do
+    Phenix.burn!
+  end
+
+  describe "Performance" do
+    before do
+      CreateWithNoParams.up
+      CreateWithoutGlobalUIDs.up
+    end
+
+    it "has a negligible performance impact" do
+      report = Benchmark.ips do |x|
+
+        x.report(WithGlobalUID)    { WithGlobalUID.create! }
+        x.report(WithoutGlobalUID) { WithoutGlobalUID.create! }
+
+
+        x.compare!
+      end
+
+      with_global_uid    = report.entries.find { |e| e.label == WithGlobalUID }.stats
+      without_global_uid = report.entries.find { |e| e.label == WithoutGlobalUID }.stats
+
+      performance_impact, _ = with_global_uid.slowdown(without_global_uid)
+      # assert_operator performance_impact, :<, 1.45
+    end
+
+    after do
+      reset_connections!
+      CreateWithNoParams.down
+      CreateWithoutGlobalUIDs.down
+    end
+  end
+end

test/models.rb → test/support/models.rb

@@ -3,6 +3,7 @@ class WithGlobalUID < ActiveRecord::Base
 end
 
 class WithoutGlobalUID < ActiveRecord::Base
+  disable_global_uid
 end
 
 class Parent < ActiveRecord::Base

test/test_helper.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 require 'bundler/setup'
 require "active_record"
+require 'benchmark/ips'
 require 'minitest/autorun'
 require 'minitest/rg'
 require 'minitest/line/describe_track'
@@ -9,20 +10,42 @@
 require 'phenix'
 require 'pry'
 
-GlobalUid::Base.global_uid_options = {
-  :disabled   => false,
-  :id_servers => [
-    "test_id_server_1",
-    "test_id_server_2"
-  ]
-}
+require_relative 'support/migrations'
+require_relative 'support/models'
 
 Phenix.configure do |config|
   config.database_config_path = File.join(File.dirname(__FILE__), "config/database.yml")
 end
 
 Phenix.rise!(with_schema: false)
 ActiveRecord::Base.establish_connection(:test)
-ActiveRecord::Base.logger = Logger.new(File.dirname(__FILE__) + "/test.log")
+ActiveRecord::Base.logger = Logger.new(File.join(File.dirname(__FILE__), "test.log"))
 ActiveSupport.test_order = :sorted if ActiveSupport.respond_to?(:test_order=)
 ActiveRecord::Migration.verbose = false
+
+def test_unique_ids(amount = 10)
+  amount.times.each_with_object({}) do |_, seen|
+    record = WithGlobalUID.create!
+    refute_nil record.id
+    assert_nil record.description
+    refute seen.has_key?(record.id)
+    seen[record.id] = true
+  end
+end
+
+def reset_connections!
+  GlobalUid::Base.servers = nil
+end
+
+def restore_defaults!
+  GlobalUid::Base.global_uid_options = {
+    :id_servers => [
+      "test_id_server_1",
+      "test_id_server_2"
+    ]
+  }
+
+  # Randomize connections for test processes to ensure they're not
+  # sticky during tests
+  GlobalUid::Base.global_uid_options[:per_process_affinity] = false
+end