From 8021da44739c0b0bf0ddffdd3a29b0625ecdb668 Mon Sep 17 00:00:00 2001 From: Stanislav Katkov Date: Mon, 9 Oct 2023 21:48:24 +0200 Subject: [PATCH 1/4] implement support for hash index --- lib/solid_cache/store/api.rb | 5 +++++ test/unit/delete_matched_test.rb | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/lib/solid_cache/store/api.rb b/lib/solid_cache/store/api.rb index abde5a4..95e8e76 100644 --- a/lib/solid_cache/store/api.rb +++ b/lib/solid_cache/store/api.rb @@ -16,6 +16,7 @@ def delete_matched(matcher, options = {}) instrument :delete_matched, matcher do raise ArgumentError, "Only strings are supported: #{matcher.inspect}" unless String === matcher raise ArgumentError, "Strings cannot start with wildcards" if SQL_WILDCARD_CHARS.include?(matcher[0]) + raise NotImplementedError, "Primary Key uses a Hash Index, delete_matched method is not supported in this case" if primary_key_using_hash_index? options ||= {} batch_size = options.fetch(:batch_size, 1000) @@ -49,6 +50,10 @@ def clear(options = nil) end private + def primary_key_using_hash_index? + ActiveRecord::Base.connection.indexes(:activesupport_cache_entries).first {|i| i.columns.first == 'key'}.using == :hash + end + def read_entry(key, **options) deserialize_entry(read_serialized_entry(key, **options), **options) end diff --git a/test/unit/delete_matched_test.rb b/test/unit/delete_matched_test.rb index c055dd3..38f6f13 100644 --- a/test/unit/delete_matched_test.rb +++ b/test/unit/delete_matched_test.rb @@ -51,6 +51,13 @@ class DeleteMatchedTest < ActiveSupport::TestCase assert @cache.exist?(other_key) end + test "will raise error if hash index is used" do + # TODO: change migration to have hash index. + assert_raise NotImplementedError do + @cache.delete_matched("foo") + end + end + test "fails when starts with %" do assert_raise ArgumentError do @cache.delete_matched("%foo") From 8e1bb5d1ac709b04a55c2dad20c5ddad8b6bbae9 Mon Sep 17 00:00:00 2001 From: Stanislav Katkov Date: Mon, 9 Oct 2023 22:45:18 +0200 Subject: [PATCH 2/4] ability to change index to hash --- README.md | 10 +++++++++- ...0230724121448_create_solid_cache_entries.rb | 11 ----------- .../solid_cache/install/install_generator.rb | 18 ++++++++++++++---- .../create_solid_cache_entries_btree.rb | 12 ++++++++++++ .../create_solid_cache_entries_hash.rb | 12 ++++++++++++ lib/tasks/solid_cache_tasks.rake | 10 ++++++++-- test/unit/delete_matched_test.rb | 7 ------- 7 files changed, 55 insertions(+), 25 deletions(-) delete mode 100644 db/migrate/20230724121448_create_solid_cache_entries.rb create mode 100644 lib/generators/solid_cache/install/templates/create_solid_cache_entries_btree.rb create mode 100644 lib/generators/solid_cache/install/templates/create_solid_cache_entries_hash.rb diff --git a/README.md b/README.md index bb9f7c4..87efc34 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,15 @@ $ gem install solid_cache Add the migration to your app: ```bash -$ bin/rails solid_cache:install:migrations +$ rake solid_cache:install +``` + +By default, this migration uses a btree index which descent through index to find a right entry. Hash index offers a faster record lookup, it’s O(1) operation that should improve lookup performance by 40-60%. But main drawback is that #delete_matched method will not be operational with hash index. + +In case, you want to go with hash index run the following instead: + +```bash +$ rake solid_cache:install[hash] ``` Then run it: diff --git a/db/migrate/20230724121448_create_solid_cache_entries.rb b/db/migrate/20230724121448_create_solid_cache_entries.rb deleted file mode 100644 index 82749d9..0000000 --- a/db/migrate/20230724121448_create_solid_cache_entries.rb +++ /dev/null @@ -1,11 +0,0 @@ -class CreateSolidCacheEntries < ActiveRecord::Migration[7.0] - def change - create_table :solid_cache_entries do |t| - t.binary :key, null: false, limit: 1024 - t.binary :value, null: false, limit: 512.megabytes - t.datetime :created_at, null: false - - t.index :key, unique: true - end - end -end diff --git a/lib/generators/solid_cache/install/install_generator.rb b/lib/generators/solid_cache/install/install_generator.rb index 6b649be..f04b676 100644 --- a/lib/generators/solid_cache/install/install_generator.rb +++ b/lib/generators/solid_cache/install/install_generator.rb @@ -1,6 +1,11 @@ +require 'rails/generators/active_record' + class SolidCache::InstallGenerator < Rails::Generators::Base - class_option :skip_migrations, type: :boolean, default: nil, - desc: "Skip migrations" + include ActiveRecord::Generators::Migration + + source_root File.expand_path("templates", __dir__) + class_option :skip_migrations, type: :boolean, default: nil, desc: "Skip migrations" + class_option :index, type: :string, default: 'btree', desc: "Index type for key column" def add_rails_cache %w[development test production].each do |env_name| @@ -11,8 +16,13 @@ def add_rails_cache end def create_migrations - unless options[:skip_migrations] - rails_command "railties:install:migrations FROM=solid_cache", inline: true + return if options[:skip_migrations] + + case options[:index] + when 'btree' + migration_template 'create_solid_cache_entries_btree.rb', 'db/migrate/create_solid_cache_entries.rb' + when 'hash' + migration_template 'create_solid_cache_entries_hash.rb', 'db/migrate/create_solid_cache_entries.rb' end end end diff --git a/lib/generators/solid_cache/install/templates/create_solid_cache_entries_btree.rb b/lib/generators/solid_cache/install/templates/create_solid_cache_entries_btree.rb new file mode 100644 index 0000000..2b7128d --- /dev/null +++ b/lib/generators/solid_cache/install/templates/create_solid_cache_entries_btree.rb @@ -0,0 +1,12 @@ +class CreateSolidCacheEntries < ActiveRecord::Migration[7.0] + def change + create_table :solid_cache_entries do |t| + t.binary :key, null: false, limit: 1024 + t.binary :value, null: false, limit: 512.megabytes + t.datetime :created_at, null: false + + t.index :key, unique: true + end + end + end + \ No newline at end of file diff --git a/lib/generators/solid_cache/install/templates/create_solid_cache_entries_hash.rb b/lib/generators/solid_cache/install/templates/create_solid_cache_entries_hash.rb new file mode 100644 index 0000000..540190e --- /dev/null +++ b/lib/generators/solid_cache/install/templates/create_solid_cache_entries_hash.rb @@ -0,0 +1,12 @@ +class CreateSolidCacheEntries < ActiveRecord::Migration[7.0] + def change + create_table :solid_cache_entries do |t| + t.binary :key, null: false, limit: 1024 + t.binary :value, null: false, limit: 512.megabytes + t.datetime :created_at, null: false + + t.index :key, unique: true, using: :hash + end + end + end + \ No newline at end of file diff --git a/lib/tasks/solid_cache_tasks.rake b/lib/tasks/solid_cache_tasks.rake index 7f3280c..140e41a 100644 --- a/lib/tasks/solid_cache_tasks.rake +++ b/lib/tasks/solid_cache_tasks.rake @@ -1,6 +1,12 @@ desc "Copy over the migration, and set cache" namespace :solid_cache do - task :install do - Rails::Command.invoke :generate, [ "solid_cache:install" ] + task :install, [:index] do |_t, args| + args.with_defaults(index: 'btree') + + if args[:index] == 'btree' || args[:index] == 'hash' + Rails::Command.invoke :generate, [ "solid_cache:install", "--index=#{args[:index]}" ] + else + abort "Invalid index type - only 'btree' and 'hash' are supported." + end end end diff --git a/test/unit/delete_matched_test.rb b/test/unit/delete_matched_test.rb index 38f6f13..c055dd3 100644 --- a/test/unit/delete_matched_test.rb +++ b/test/unit/delete_matched_test.rb @@ -51,13 +51,6 @@ class DeleteMatchedTest < ActiveSupport::TestCase assert @cache.exist?(other_key) end - test "will raise error if hash index is used" do - # TODO: change migration to have hash index. - assert_raise NotImplementedError do - @cache.delete_matched("foo") - end - end - test "fails when starts with %" do assert_raise ArgumentError do @cache.delete_matched("%foo") From 4c573579c91bace15b9d1cf83ac31e95606672c1 Mon Sep 17 00:00:00 2001 From: Stanislav Katkov Date: Mon, 9 Oct 2023 23:11:12 +0200 Subject: [PATCH 3/4] fmt --- .../solid_cache/install/install_generator.rb | 16 ++++++++-------- .../create_solid_cache_entries_btree.rb | 5 ++--- .../templates/create_solid_cache_entries_hash.rb | 5 ++--- lib/solid_cache/store/api.rb | 2 +- lib/tasks/solid_cache_tasks.rake | 6 +++--- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/generators/solid_cache/install/install_generator.rb b/lib/generators/solid_cache/install/install_generator.rb index f04b676..99bc312 100644 --- a/lib/generators/solid_cache/install/install_generator.rb +++ b/lib/generators/solid_cache/install/install_generator.rb @@ -1,11 +1,11 @@ -require 'rails/generators/active_record' +require "rails/generators/active_record" class SolidCache::InstallGenerator < Rails::Generators::Base include ActiveRecord::Generators::Migration - + source_root File.expand_path("templates", __dir__) class_option :skip_migrations, type: :boolean, default: nil, desc: "Skip migrations" - class_option :index, type: :string, default: 'btree', desc: "Index type for key column" + class_option :index, type: :string, default: "btree", desc: "Index type for key column" def add_rails_cache %w[development test production].each do |env_name| @@ -17,12 +17,12 @@ def add_rails_cache def create_migrations return if options[:skip_migrations] - + case options[:index] - when 'btree' - migration_template 'create_solid_cache_entries_btree.rb', 'db/migrate/create_solid_cache_entries.rb' - when 'hash' - migration_template 'create_solid_cache_entries_hash.rb', 'db/migrate/create_solid_cache_entries.rb' + when "btree" + migration_template "create_solid_cache_entries_btree.rb", "db/migrate/create_solid_cache_entries.rb" + when "hash" + migration_template "create_solid_cache_entries_hash.rb", "db/migrate/create_solid_cache_entries.rb" end end end diff --git a/lib/generators/solid_cache/install/templates/create_solid_cache_entries_btree.rb b/lib/generators/solid_cache/install/templates/create_solid_cache_entries_btree.rb index 2b7128d..39be9f5 100644 --- a/lib/generators/solid_cache/install/templates/create_solid_cache_entries_btree.rb +++ b/lib/generators/solid_cache/install/templates/create_solid_cache_entries_btree.rb @@ -4,9 +4,8 @@ def change t.binary :key, null: false, limit: 1024 t.binary :value, null: false, limit: 512.megabytes t.datetime :created_at, null: false - + t.index :key, unique: true end end - end - \ No newline at end of file +end diff --git a/lib/generators/solid_cache/install/templates/create_solid_cache_entries_hash.rb b/lib/generators/solid_cache/install/templates/create_solid_cache_entries_hash.rb index 540190e..d87e1af 100644 --- a/lib/generators/solid_cache/install/templates/create_solid_cache_entries_hash.rb +++ b/lib/generators/solid_cache/install/templates/create_solid_cache_entries_hash.rb @@ -4,9 +4,8 @@ def change t.binary :key, null: false, limit: 1024 t.binary :value, null: false, limit: 512.megabytes t.datetime :created_at, null: false - + t.index :key, unique: true, using: :hash end end - end - \ No newline at end of file +end diff --git a/lib/solid_cache/store/api.rb b/lib/solid_cache/store/api.rb index 95e8e76..acab50f 100644 --- a/lib/solid_cache/store/api.rb +++ b/lib/solid_cache/store/api.rb @@ -51,7 +51,7 @@ def clear(options = nil) private def primary_key_using_hash_index? - ActiveRecord::Base.connection.indexes(:activesupport_cache_entries).first {|i| i.columns.first == 'key'}.using == :hash + ActiveRecord::Base.connection.indexes(:activesupport_cache_entries).first { |i| i.columns.first == "key" }.using == :hash end def read_entry(key, **options) diff --git a/lib/tasks/solid_cache_tasks.rake b/lib/tasks/solid_cache_tasks.rake index 140e41a..c97a6f2 100644 --- a/lib/tasks/solid_cache_tasks.rake +++ b/lib/tasks/solid_cache_tasks.rake @@ -1,9 +1,9 @@ desc "Copy over the migration, and set cache" namespace :solid_cache do - task :install, [:index] do |_t, args| - args.with_defaults(index: 'btree') + task :install, [ :index ] do |_t, args| + args.with_defaults(index: "btree") - if args[:index] == 'btree' || args[:index] == 'hash' + if args[:index] == "btree" || args[:index] == "hash" Rails::Command.invoke :generate, [ "solid_cache:install", "--index=#{args[:index]}" ] else abort "Invalid index type - only 'btree' and 'hash' are supported." From 915e8f813767e96f1e5a0f455f55c2a23112dfb3 Mon Sep 17 00:00:00 2001 From: Stanislav Katkov Date: Tue, 10 Oct 2023 21:00:22 +0200 Subject: [PATCH 4/4] CI: don't fail fast --- .github/workflows/main.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 0a6265d..07d34ec 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -19,6 +19,7 @@ jobs: name: Tests runs-on: ubuntu-latest strategy: + fail-fast: false matrix: ruby-version: [3.2.2] database: [sqlite, postgres, mysql]