diff --git a/.gitlab/ci/package-and-test/main.gitlab-ci.yml b/.gitlab/ci/package-and-test/main.gitlab-ci.yml index 0d30cb78be..b1c04ba8a1 100644 --- a/.gitlab/ci/package-and-test/main.gitlab-ci.yml +++ b/.gitlab/ci/package-and-test/main.gitlab-ci.yml @@ -124,6 +124,9 @@ trigger-omnibus-env: echo "OMNIBUS_GITLAB_RUBY3_BUILD=${OMNIBUS_GITLAB_RUBY3_BUILD:-false}" >> $BUILD_ENV echo "OMNIBUS_GITLAB_CACHE_EDITION=${OMNIBUS_GITLAB_CACHE_EDITION:-GITLAB}" >> $BUILD_ENV echo "GITLAB_ASSETS_TAG=$(assets_image_tag)" >> $BUILD_ENV + echo "EE=$([[ $FOSS_ONLY == '1' ]] && echo 'false' || echo 'true')" >> $BUILD_ENV + target_branch_name="${CI_MERGE_REQUEST_TARGET_BRANCH_NAME:-${CI_COMMIT_REF_NAME}}" + echo "TRIGGER_BRANCH=$([[ "${target_branch_name}" =~ ^[0-9-]+-stable(-ee)?$ ]] && echo ${target_branch_name%-ee} || echo 'master')" >> $BUILD_ENV echo "Built environment file for omnibus build:" cat $BUILD_ENV artifacts: @@ -155,9 +158,10 @@ trigger-omnibus: CACHE_EDITION: $OMNIBUS_GITLAB_CACHE_EDITION SKIP_QA_DOCKER: "true" SKIP_QA_TEST: "true" - ee: "true" + ee: $EE trigger: project: gitlab-org/build/omnibus-gitlab-mirror + branch: $TRIGGER_BRANCH strategy: depend download-knapsack-report: diff --git a/CHANGELOG.md b/CHANGELOG.md index bfe402bef6..ce67af5c52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 15.9.8 (2023-05-10) + +No changes. + ## 15.9.7 (2023-05-03) ### Security (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 916b312da4..63e48abe4e 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -15.9.7 \ No newline at end of file +15.9.8 \ No newline at end of file diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 916b312da4..63e48abe4e 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -15.9.7 \ No newline at end of file +15.9.8 \ No newline at end of file diff --git a/VERSION b/VERSION index 916b312da4..63e48abe4e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -15.9.7 \ No newline at end of file +15.9.8 \ No newline at end of file diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 485ca3a385..f3488f6ea6 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1478,6 +1478,7 @@ class MergeRequest < ApplicationRecord def fetch_ref! target_project.repository.fetch_source_branch!(source_project.repository, source_branch, ref_path) + expire_ancestor_cache end # Returns the current merge-ref HEAD commit. @@ -2037,6 +2038,10 @@ class MergeRequest < ApplicationRecord self.draft = draft? end + def expire_ancestor_cache + project.repository.expire_ancestor_cache(target_branch_sha, diff_head_sha) + end + def missing_report_error(report_type) { status: :error, status_reason: "This merge request does not have #{report_type} reports" } end diff --git a/app/models/repository.rb b/app/models/repository.rb index d15f2a430f..f951418c0b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -996,7 +996,7 @@ class Repository def ancestor?(ancestor_id, descendant_id) return false if ancestor_id.nil? || descendant_id.nil? - cache_key = "ancestor:#{ancestor_id}:#{descendant_id}" + cache_key = ancestor_cache_key(ancestor_id, descendant_id) request_store_cache.fetch(cache_key) do cache.fetch(cache_key) do raw_repository.ancestor?(ancestor_id, descendant_id) @@ -1004,6 +1004,12 @@ class Repository end end + def expire_ancestor_cache(ancestor_id, descendant_id) + cache_key = ancestor_cache_key(ancestor_id, descendant_id) + request_store_cache.expire(cache_key) + cache.expire(cache_key) + end + def clone_as_mirror(url, http_authorization_header: "", resolved_address: "") import_repository(url, http_authorization_header: http_authorization_header, mirror: true, resolved_address: resolved_address) end @@ -1224,6 +1230,10 @@ class Repository private + def ancestor_cache_key(ancestor_id, descendant_id) + "ancestor:#{ancestor_id}:#{descendant_id}" + end + # TODO Genericize finder, later split this on finders by Ref or Oid # https://gitlab.com/gitlab-org/gitlab/issues/19877 def find_commit(oid_or_ref) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 2e2355ba71..a3c0c9a0a7 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -4239,10 +4239,14 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev subject { create(:merge_request, source_project: project) } - it 'fetches the ref correctly' do + it 'fetches the ref and expires the ancestor cache' do expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error + expect(project.repository).to receive(:expire_ancestor_cache).with(subject.target_branch_sha, subject.diff_head_sha).and_call_original + expect(subject).to receive(:expire_ancestor_cache).and_call_original + subject.fetch_ref! + expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy end end @@ -4253,7 +4257,8 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev # We use build instead of create to test that an IID is allocated subject { build(:merge_request, source_project: project) } - it 'fetches the ref correctly' do + it 'fetches the ref and expires the ancestor cache' do + expect(subject).to receive(:expire_ancestor_cache).and_call_original expect(subject.iid).to be_nil expect { subject.eager_fetch_ref! }.to change { subject.iid.to_i }.by(1) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index b8780b3faa..44fcf87526 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -3078,6 +3078,18 @@ RSpec.describe Repository, feature_category: :source_code_management do 2.times { repository.ancestor?(commit.id, ancestor.id) } end + it 'calls out to Gitaly again after expiration' do + expect(repository.raw_repository).to receive(:ancestor?).once + + repository.ancestor?(commit.id, ancestor.id) + + repository.expire_ancestor_cache(commit.id, ancestor.id) + + expect(repository.raw_repository).to receive(:ancestor?).once + + 2.times { repository.ancestor?(commit.id, ancestor.id) } + end + it 'returns the value from the request store' do repository.__send__(:request_store_cache).write(cache_key, "it's apparent") diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb index 44fe9063ac..9e811eaa25 100644 --- a/spec/services/draft_notes/publish_service_spec.rb +++ b/spec/services/draft_notes/publish_service_spec.rb @@ -172,7 +172,12 @@ RSpec.describe DraftNotes::PublishService do end end - it 'does not requests a lot from Gitaly', :request_store do + it 'does not request a lot from Gitaly', :request_store, :clean_gitlab_redis_cache do + merge_request + position + + Gitlab::GitalyClient.reset_counts + # NOTE: This should be reduced as we work on reducing Gitaly calls. # Gitaly requests shouldn't go above this threshold as much as possible # as it may add more to the Gitaly N+1 issue we are experiencing. diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index 3d5b65207e..6cf8af1fcf 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -34,8 +34,9 @@ RSpec.describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_ context 'cache clearing' do it 'clears the cache for older diffs on the merge request' do - expect_any_instance_of(Redis).to receive(:del).once.and_call_original - expect(Rails.cache).to receive(:delete).once.and_call_original + expect_next_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff) do |instance| + expect(instance).to receive(:clear_cache).and_call_original + end subject.execute end