diff --git a/app/models/concerns/ci/partitionable.rb b/app/models/concerns/ci/partitionable.rb index 03cce1edf7445d4870e33c062619e57b452e7fb9..679107363bd77846ad72571afec546abb05306ae 100644 --- a/app/models/concerns/ci/partitionable.rb +++ b/app/models/concerns/ci/partitionable.rb @@ -75,7 +75,7 @@ def handle_partitionable_ddl(partitioned) partitioned_by :partition_id, strategy: :ci_sliding_list, next_partition_if: ->(latest_partition) do - latest_partition.blank? || Ci::Pipeline::NEXT_PARTITION_VALUE > latest_partition.value + latest_partition.blank? || Ci::Pipeline::NEXT_PARTITION_VALUE > latest_partition.values.max end, detach_partition_if: proc { false }, # Most of the db tasks are run in a weekly basis, e.g. execute_batched_migrations. diff --git a/lib/gitlab/database/partitioning/ci_sliding_list_strategy.rb b/lib/gitlab/database/partitioning/ci_sliding_list_strategy.rb index de6319582cb0115908b7946e3b616dfd3ad76005..70a666bc173cd57e380a7df9bca550af630b3b01 100644 --- a/lib/gitlab/database/partitioning/ci_sliding_list_strategy.rb +++ b/lib/gitlab/database/partitioning/ci_sliding_list_strategy.rb @@ -4,12 +4,18 @@ module Gitlab module Database module Partitioning class CiSlidingListStrategy < SlidingListStrategy + def current_partitions + Gitlab::Database::PostgresPartition.for_parent_table(table_name).map do |partition| + MultipleNumericListPartition.from_sql(table_name, partition.name, partition.condition) + end.sort + end + def initial_partition partition_for(100) end def next_partition - partition_for(active_partition.value + 1) + partition_for(active_partition.values.max + 1) end def missing_partitions @@ -36,7 +42,7 @@ def active_partition def ensure_partitioning_column_ignored_or_readonly!; end def partition_for(value) - SingleNumericListPartition.new(table_name, value, partition_name: partition_name(value)) + MultipleNumericListPartition.new(table_name, value, partition_name: partition_name(value)) end def partition_name(value) diff --git a/lib/gitlab/database/partitioning/multiple_numeric_list_partition.rb b/lib/gitlab/database/partitioning/multiple_numeric_list_partition.rb new file mode 100644 index 0000000000000000000000000000000000000000..b3c8575cbd122f27ae6ff748698ee2dd37f89ec3 --- /dev/null +++ b/lib/gitlab/database/partitioning/multiple_numeric_list_partition.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Partitioning + class MultipleNumericListPartition + include Comparable + + def self.from_sql(table, partition_name, definition) + matches = definition.match(/\AFOR VALUES IN \((?<values>.*)\)\z/) + + raise ArgumentError, 'Unknown partition definition' unless matches + + values = matches[:values].scan(/\d+/).map { |value| Integer(value) } + + new(table, values, partition_name: partition_name) + end + + attr_reader :table, :values + + def initialize(table, values, partition_name: nil) + @table = table + @values = Array.wrap(values) + @partition_name = partition_name + end + + def partition_name + @partition_name || ([table] + values).join('_') + end + + def data_size + execute("SELECT pg_table_size(#{quote(full_partition_name)})").first['pg_table_size'] + end + + def to_sql + <<~SQL.squish + CREATE TABLE IF NOT EXISTS #{fully_qualified_partition} + PARTITION OF #{quote_table_name(table)} + FOR VALUES IN (#{quoted_values}) + SQL + end + + def to_detach_sql + <<~SQL.squish + ALTER TABLE #{quote_table_name(table)} + DETACH PARTITION #{fully_qualified_partition} + SQL + end + + def ==(other) + table == other.table && + partition_name == other.partition_name && + values == other.values + end + alias_method :eql?, :== + + def hash + [table, partition_name, values].hash + end + + def <=>(other) + return if table != other.table + + values <=> other.values + end + + private + + delegate :execute, :quote, :quote_table_name, to: :conn, private: true + + def full_partition_name + format("%s.%s", Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA, partition_name) + end + + def fully_qualified_partition + quote_table_name(full_partition_name) + end + + def quoted_values + values.map { |value| quote(value) }.join(', ') + end + + def conn + @conn ||= Gitlab::Database::SharedModel.connection + end + end + end + end +end diff --git a/spec/lib/gitlab/database/partitioning/ci_sliding_list_strategy_spec.rb b/spec/lib/gitlab/database/partitioning/ci_sliding_list_strategy_spec.rb index 337749446ed81114c1c226dde01d0e3e9d27f70f..0f306b15c605134718cd2692c8a4be3c1c5c8504 100644 --- a/spec/lib/gitlab/database/partitioning/ci_sliding_list_strategy_spec.rb +++ b/spec/lib/gitlab/database/partitioning/ci_sliding_list_strategy_spec.rb @@ -40,14 +40,38 @@ it 'detects both partitions' do expect(strategy.current_partitions).to eq( [ - Gitlab::Database::Partitioning::SingleNumericListPartition.new( + Gitlab::Database::Partitioning::MultipleNumericListPartition.new( table_name, 100, partition_name: "#{table_name}_100" ), - Gitlab::Database::Partitioning::SingleNumericListPartition.new( + Gitlab::Database::Partitioning::MultipleNumericListPartition.new( table_name, 101, partition_name: "#{table_name}_101" ) ]) end + + context 'with multiple values' do + before do + connection.execute(<<~SQL) + create table #{table_name}_test + partition of #{table_name} for values in (102, 103, 104); + SQL + end + + it 'detects all partitions' do + expect(strategy.current_partitions).to eq( + [ + Gitlab::Database::Partitioning::MultipleNumericListPartition.new( + table_name, 100, partition_name: "#{table_name}_100" + ), + Gitlab::Database::Partitioning::MultipleNumericListPartition.new( + table_name, 101, partition_name: "#{table_name}_101" + ), + Gitlab::Database::Partitioning::MultipleNumericListPartition.new( + table_name, [102, 103, 104], partition_name: "#{table_name}_test" + ) + ]) + end + end end describe '#validate_and_fix' do @@ -60,7 +84,7 @@ describe '#active_partition' do it 'is the partition with the largest value' do - expect(strategy.active_partition.value).to eq(101) + expect(strategy.active_partition.values).to eq([101]) end context 'when there are no partitions' do @@ -69,7 +93,7 @@ end it 'is the initial partition' do - expect(strategy.active_partition.value).to eq(100) + expect(strategy.active_partition.values).to eq([100]) end end end @@ -82,7 +106,7 @@ extra = strategy.missing_partitions expect(extra.length).to eq(1) - expect(extra.first.value).to eq(102) + expect(extra.first.values).to eq([102]) end context 'when there are no partitions for the table' do @@ -92,7 +116,7 @@ missing_partitions = strategy.missing_partitions expect(missing_partitions.size).to eq(2) - expect(missing_partitions.map(&:value)).to match_array([100, 101]) + expect(missing_partitions.map(&:values)).to match_array([[100], [101]]) end end end @@ -114,7 +138,7 @@ expect(missing_partitions.size).to eq(1) missing_partition = missing_partitions.first - expect(missing_partition.value).to eq(100) + expect(missing_partition.values).to eq([100]) end end end @@ -136,7 +160,7 @@ describe '#initial_partition' do it 'starts with the value 100', :aggregate_failures do initial_partition = strategy.initial_partition - expect(initial_partition.value).to eq(100) + expect(initial_partition.values).to eq([100]) expect(initial_partition.table).to eq(strategy.table_name) expect(initial_partition.partition_name).to eq("#{strategy.table_name}_100") end @@ -147,7 +171,7 @@ it 'removes the prefix', :aggregate_failures do initial_partition = strategy.initial_partition - expect(initial_partition.value).to eq(100) + expect(initial_partition.values).to eq([100]) expect(initial_partition.table).to eq(strategy.table_name) expect(initial_partition.partition_name).to eq('test_gitlab_ci_partitioned_test_100') end @@ -158,13 +182,13 @@ before do allow(strategy) .to receive(:active_partition) - .and_return(instance_double(Gitlab::Database::Partitioning::SingleNumericListPartition, value: 105)) + .and_return(instance_double(Gitlab::Database::Partitioning::MultipleNumericListPartition, values: [105])) end it 'is one after the active partition', :aggregate_failures do next_partition = strategy.next_partition - expect(next_partition.value).to eq(106) + expect(next_partition.values).to eq([106]) expect(next_partition.table).to eq(strategy.table_name) expect(next_partition.partition_name).to eq("#{strategy.table_name}_106") end @@ -175,7 +199,7 @@ it 'removes the prefix', :aggregate_failures do next_partition = strategy.next_partition - expect(next_partition.value).to eq(106) + expect(next_partition.values).to eq([106]) expect(next_partition.table).to eq(strategy.table_name) expect(next_partition.partition_name).to eq('test_gitlab_ci_partitioned_test_106') end diff --git a/spec/lib/gitlab/database/partitioning/multiple_numeric_list_partition_spec.rb b/spec/lib/gitlab/database/partitioning/multiple_numeric_list_partition_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0d8f6732466f74e04b59637fa0cffb28f72a8e5c --- /dev/null +++ b/spec/lib/gitlab/database/partitioning/multiple_numeric_list_partition_spec.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Partitioning::MultipleNumericListPartition, feature_category: :database do + describe '.from_sql' do + subject(:parsed_partition) { described_class.from_sql(table, partition_name, definition) } + + let(:table) { 'partitioned_table' } + + context 'with single partition values' do + let(:partition_value) { 10 } + let(:partition_name) { "partitioned_table_#{partition_value}" } + let(:definition) { "FOR VALUES IN ('#{partition_value}')" } + + it 'uses specified table name' do + expect(parsed_partition.table).to eq(table) + end + + it 'uses specified partition name' do + expect(parsed_partition.partition_name).to eq(partition_name) + end + + it 'parses the definition' do + expect(parsed_partition.values).to eq([partition_value]) + end + end + + context 'with multiple partition values' do + let(:partition_values) { [10, 11, 12] } + let(:partition_name) { "partitioned_table_10_11_12" } + let(:definition) { "FOR VALUES IN ('10', '11', '12')" } + + it 'uses specified table name' do + expect(parsed_partition.table).to eq(table) + end + + it 'uses specified partition name' do + expect(parsed_partition.partition_name).to eq(partition_name) + end + + it 'parses the definition' do + expect(parsed_partition.values).to eq(partition_values) + end + end + end + + describe '#partition_name' do + it 'is the explicit name if provided' do + parsed_partition = described_class.new('table', 1, partition_name: 'some_other_name') + + expect(parsed_partition.partition_name).to eq('some_other_name') + end + + it 'defaults to the table name followed by the partition value' do + expect(described_class.new('table', 1).partition_name).to eq('table_1') + end + end + + describe 'sorting' do + it 'is incomparable if the tables do not match' do + expect(described_class.new('table1', 1) <=> described_class.new('table2', 2)).to be_nil + end + + it 'sorts by the value when the tables match' do + expect(described_class.new('table1', 1) <=> described_class.new('table1', 2)).to eq(1 <=> 2) + end + + it 'sorts by numeric value rather than text value' do + expect(described_class.new('table', 10)).to be > described_class.new('table', 9) + end + + it 'sorts with array values' do + expect(described_class.new('table1', + [10, 11]) <=> described_class.new('table1', [12, 13])).to eq([10, 11] <=> [12, 13]) + end + end + + describe '#hash' do + let(:data) { { described_class.new('table', 10) => 1 } } + + it { expect(data.key?(described_class.new('table', 10))).to be_truthy } + it { expect(data.key?(described_class.new('table', 9))).to be_falsey } + end + + describe '#data_size' do + it 'returns the partition size' do + partition = Gitlab::Database::PostgresPartition.for_parent_table(:p_ci_builds).last + parsed_partition = described_class.from_sql(:p_ci_builds, partition.name, partition.condition) + + expect(parsed_partition.data_size).not_to eq(0) + end + end + + describe '#to_sql' do + subject(:partition) { described_class.new('table', 10) } + + it 'generates SQL' do + sql = 'CREATE TABLE IF NOT EXISTS "gitlab_partitions_dynamic"."table_10" PARTITION OF "table" FOR VALUES IN (10)' + expect(partition.to_sql).to eq(sql) + end + end + + describe '#to_detach_sql' do + subject(:partition) { described_class.new('table', 10) } + + it 'generates SQL' do + sql = 'ALTER TABLE "table" DETACH PARTITION "gitlab_partitions_dynamic"."table_10"' + expect(partition.to_detach_sql).to eq(sql) + end + end +end diff --git a/spec/lib/gitlab/database/partitioning/partition_monitoring_spec.rb b/spec/lib/gitlab/database/partitioning/partition_monitoring_spec.rb index 006ce8a7f4813b857726df79bb176edef2ca8bd3..bbb849da39fc9af5a8e037bba5507cc2fd749749 100644 --- a/spec/lib/gitlab/database/partitioning/partition_monitoring_spec.rb +++ b/spec/lib/gitlab/database/partitioning/partition_monitoring_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::Partitioning::PartitionMonitoring do +RSpec.describe Gitlab::Database::Partitioning::PartitionMonitoring, feature_category: :database do describe '#report_metrics' do subject { described_class.new.report_metrics_for_model(model) } diff --git a/spec/lib/gitlab/database/partitioning/single_numeric_list_partition_spec.rb b/spec/lib/gitlab/database/partitioning/single_numeric_list_partition_spec.rb index 9941241e84662ddc09938e0a6fb85511ad60c1e0..eb3360f92ca1d266ee13221bb7828a79f842e4ee 100644 --- a/spec/lib/gitlab/database/partitioning/single_numeric_list_partition_spec.rb +++ b/spec/lib/gitlab/database/partitioning/single_numeric_list_partition_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::Partitioning::SingleNumericListPartition do +RSpec.describe Gitlab::Database::Partitioning::SingleNumericListPartition, feature_category: :database do describe '.from_sql' do subject(:parsed_partition) { described_class.from_sql(table, partition_name, definition) } diff --git a/spec/lib/gitlab/database/partitioning/time_partition_spec.rb b/spec/lib/gitlab/database/partitioning/time_partition_spec.rb index 5a17e8d20cf8b6064e46dd8a222c72bd035ed994..45ed0d5fcb79f10622c5e016c57db88abc53f49d 100644 --- a/spec/lib/gitlab/database/partitioning/time_partition_spec.rb +++ b/spec/lib/gitlab/database/partitioning/time_partition_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::Partitioning::TimePartition do +RSpec.describe Gitlab::Database::Partitioning::TimePartition, feature_category: :database do describe '.from_sql' do subject { described_class.from_sql(table, partition_name, definition) } diff --git a/spec/models/concerns/ci/partitionable_spec.rb b/spec/models/concerns/ci/partitionable_spec.rb index 735b81f54bcd3b21ea84cbf7a56560ccfe5d4a7f..091157ee9070a24b59b6fa93944047d71f100792 100644 --- a/spec/models/concerns/ci/partitionable_spec.rb +++ b/spec/models/concerns/ci/partitionable_spec.rb @@ -118,6 +118,16 @@ it { is_expected.to eq(false) } end + + context 'with an existing partition for partition_id in 100, 101' do + before do + ci_model.connection.execute(<<~SQL) + CREATE TABLE IF NOT EXISTS _test_table_name_101 PARTITION OF _test_table_name FOR VALUES IN (100, 101); + SQL + end + + it { is_expected.to eq(false) } + end end end