Merge branch 'docs-bprescott-pgbouncer-20200319' into 'master'

PgBouncer - going direct to master

See merge request gitlab-org/gitlab!27619

app/assets/javascripts/pipelines/components/pipeline_graph/drawing_utils.js

@@ -64,10 +64,18 @@ export const generateLinksData = ({ links }, jobs, containerID) => {
     // Start point
     path.moveTo(sourceNodeX, sourceNodeY);
 
+    // Make cross-stages lines a straight line all the way
+    // until we can safely draw the bezier to look nice.
+    const straightLineDestinationX = targetNodeX - 100;
+    const controlPointX = straightLineDestinationX + (targetNodeX - straightLineDestinationX) / 2;
+
+    if (straightLineDestinationX > 0) {
+      path.lineTo(straightLineDestinationX, sourceNodeY);
+    }
+
     // Add bezier curve. The first 4 coordinates are the 2 control
     // points to create the curve, and the last one is the end point (x, y).
     // We want our control points to be in the middle of the line
-    const controlPointX = sourceNodeX + (targetNodeX - sourceNodeX) / 2;
     path.bezierCurveTo(
       controlPointX,
       sourceNodeY,

app/assets/javascripts/sidebar/components/assignees/assignee_title.vue

 <script>
 import { GlLoadingIcon } from '@gitlab/ui';
-import { n__ } from '~/locale';
+import { n__, __ } from '~/locale';
 
 export default {
   name: 'AssigneeTitle',
@@ -26,12 +26,19 @@ export default {
       required: false,
       default: false,
     },
+    changing: {
+      type: Boolean,
+      required: true,
+    },
   },
   computed: {
     assigneeTitle() {
       const assignees = this.numberOfAssignees;
       return n__('Assignee', `%d Assignees`, assignees);
     },
+    titleCopy() {
+      return this.changing ? __('Apply') : __('Edit');
+    },
   },
 };
 </script>
@@ -43,11 +50,12 @@ export default {
       v-if="editable"
       class="js-sidebar-dropdown-toggle edit-link float-right"
       href="#"
+      data-test-id="edit-link"
       data-track-event="click_edit_button"
       data-track-label="right_sidebar"
       data-track-property="assignee"
     >
-      {{ __('Edit') }}
+      {{ titleCopy }}
     </a>
     <a
       v-if="showToggle"

app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.vue

@@ -89,6 +89,8 @@ export default {
         .saveAssignees(this.field)
         .then(() => {
           this.loading = false;
+          this.store.resetChanging();
+
           refreshUserMergeRequestCounts();
         })
         .catch(() => {
@@ -113,6 +115,7 @@ export default {
       :loading="loading || store.isFetching.assignees"
       :editable="store.editable"
       :show-toggle="!signedIn"
+      :changing="store.changing"
     />
     <assignees
       v-if="!store.isFetching.assignees"

app/assets/javascripts/sidebar/stores/sidebar_store.js

@@ -33,6 +33,7 @@ export default class SidebarStore {
     this.projectEmailsDisabled = false;
     this.subscribeDisabledDescription = '';
     this.subscribed = null;
+    this.changing = false;
 
     SidebarStore.singleton = this;
   }
@@ -51,6 +52,10 @@ export default class SidebarStore {
     }
   }
 
+  resetChanging() {
+    this.changing = false;
+  }
+
   setTimeTrackingData(data) {
     this.timeEstimate = data.time_estimate;
     this.totalTimeSpent = data.total_time_spent;
@@ -80,6 +85,7 @@ export default class SidebarStore {
 
   addAssignee(assignee) {
     if (!this.findAssignee(assignee)) {
+      this.changing = true;
       this.assignees.push(assignee);
     }
   }
@@ -100,6 +106,7 @@ export default class SidebarStore {
 
   removeAssignee(assignee) {
     if (assignee) {
+      this.changing = true;
       this.assignees = this.assignees.filter(({ id }) => id !== assignee.id);
     }
   }
@@ -111,6 +118,7 @@ export default class SidebarStore {
   }
 
   removeAllAssignees() {
+    this.changing = true;
     this.assignees = [];
   }
 

app/models/application_record.rb

@@ -61,4 +61,8 @@ class ApplicationRecord < ActiveRecord::Base
   def self.underscore
     Gitlab::SafeRequestStore.fetch("model:#{self}:underscore") { self.to_s.underscore }
   end
+
+  def self.where_exists(query)
+    where('EXISTS (?)', query.select(1))
+  end
 end

app/models/ci/build_trace_chunk.rb

@@ -136,6 +136,10 @@ module Ci
     # We are using optimistic locking combined with Redis locking to ensure
     # that a chunk gets migrated properly.
     #
+    # We are catching an exception related to an exclusive lock not being
+    # acquired because it is creating a lot of noise, and is a result of
+    # duplicated workers running in parallel for the same build trace chunk.
+    #
     def persist_data!
       in_lock(*lock_params) do         # exclusive Redis lock is acquired first
         raise FailedToPersistDataError, 'Modifed build trace chunk detected' if has_changes_to_save?
@@ -144,6 +148,8 @@ module Ci
           chunk.unsafe_persist_data!   # we migrate the data and update data store
         end
       end
+    rescue FailedToObtainLockError
+      metrics.increment_trace_operation(operation: :stalled)
     rescue ActiveRecord::StaleObjectError
       raise FailedToPersistDataError, <<~MSG
         Data migration race condition detected

changelogs/unreleased/ss-add-apply-to-assignees.yml

+---
+title: Add apply button when user changes assignees
+merge_request: 44812
+author:
+type: added

doc/administration/postgresql/replication_and_failover.md

@@ -1445,7 +1445,7 @@ Considering these, you should carefully plan your PostgreSQL upgrade:
    sudo gitlab-ctl pg-upgrade -V 12
    ```
 
-CAUTION: **Warning:**
+NOTE: **Note:**
 Reverting PostgreSQL upgrade with `gitlab-ctl revert-pg-upgrade` has the same considerations as
-`gitlab-ctl pg-upgrade`. It can be complicated and may involve deletion of the data directory.
-If you need to do that, please contact GitLab support.
+`gitlab-ctl pg-upgrade`. You should follow the same procedure by first stopping the replicas,
+then reverting the leader, and finally reverting the replicas.

ee/spec/features/boards/swimlanes/epics_swimlanes_filtering_spec.rb

@@ -61,7 +61,7 @@ RSpec.describe 'epics swimlanes filtering', :js do
       wait_for_empty_boards((3..4))
     end
 
-    it 'filters by assignee' do
+    it 'filters by assignee', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/266990' do
       wait_for_all_requests
 
       set_filter("assignee", user.username)

lib/gitlab/ci/trace/metrics.rb

@@ -6,9 +6,20 @@ module Gitlab
       class Metrics
         extend Gitlab::Utils::StrongMemoize
 
-        OPERATIONS = [:appended, :streamed, :chunked, :mutated, :overwrite,
-                      :accepted, :finalized, :discarded, :conflict, :locked,
-                      :invalid].freeze
+        OPERATIONS = [
+          :appended,  # new trace data has been written to a chunk
+          :streamed,  # new trace data has been sent by a runner
+          :chunked,   # new trace chunk has been created
+          :mutated,   # trace has been mutated when removing secrets
+          :overwrite, # runner requested overwritting a build trace
+          :accepted,  # scheduled chunks for migration and responded with 202
+          :finalized, # all live build trace chunks have been persisted
+          :discarded, # failed to persist live chunks before timeout
+          :conflict,  # runner has sent unrecognized build state details
+          :locked,    # build trace has been locked by a different mechanism
+          :stalled,   # failed to migrate chunk due to a worker duplication
+          :invalid    # malformed build trace has been detected using CRC32
+        ].freeze
 
         def increment_trace_operation(operation: :unknown)
           unless OPERATIONS.include?(operation)

spec/features/issues/issue_sidebar_spec.rb

@@ -140,6 +140,18 @@ RSpec.describe 'Issue Sidebar' do
         end
       end
     end
+
+    it 'shows label text as "Apply" when assignees are changed' do
+      project.add_developer(user)
+      visit_issue(project, issue2)
+
+      find('.block.assignee .edit-link').click
+      wait_for_requests
+
+      click_on 'Unassigned'
+
+      expect(page).to have_link('Apply')
+    end
   end
 
   context 'as a allowed user' do

spec/frontend/sidebar/assignee_title_spec.js

@@ -11,6 +11,7 @@ describe('AssigneeTitle component', () => {
       propsData: {
         numberOfAssignees: 0,
         editable: false,
+        changing: false,
         ...props,
       },
     });
@@ -62,6 +63,22 @@ describe('AssigneeTitle component', () => {
     });
   });
 
+  describe('when changing is false', () => {
+    it('renders "Edit"', () => {
+      wrapper = createComponent({ editable: true });
+
+      expect(wrapper.find('[data-test-id="edit-link"]').text()).toEqual('Edit');
+    });
+  });
+
+  describe('when changing is true', () => {
+    it('renders "Edit"', () => {
+      wrapper = createComponent({ editable: true, changing: true });
+
+      expect(wrapper.find('[data-test-id="edit-link"]').text()).toEqual('Apply');
+    });
+  });
+
   it('does not render spinner by default', () => {
     wrapper = createComponent({
       numberOfAssignees: 0,

spec/frontend/sidebar/sidebar_assignees_spec.js

@@ -20,6 +20,7 @@ describe('sidebar assignees', () => {
         mediator,
         field: '',
         projectPath: 'projectPath',
+        changing: false,
         ...props,
       },
       provide: {

spec/frontend/sidebar/sidebar_store_spec.js

@@ -57,16 +57,40 @@ describe('Sidebar store', () => {
     expect(testContext.store.isFetching.assignees).toBe(true);
   });
 
-  it('adds a new assignee', () => {
-    testContext.store.addAssignee(ASSIGNEE);
+  it('resets changing when resetChanging is called', () => {
+    testContext.store.changing = true;
+
+    testContext.store.resetChanging();
 
-    expect(testContext.store.assignees.length).toEqual(1);
+    expect(testContext.store.changing).toBe(false);
   });
 
-  it('removes an assignee', () => {
-    testContext.store.removeAssignee(ASSIGNEE);
+  describe('when it adds a new assignee', () => {
+    beforeEach(() => {
+      testContext.store.addAssignee(ASSIGNEE);
+    });
 
-    expect(testContext.store.assignees.length).toEqual(0);
+    it('adds a new assignee', () => {
+      expect(testContext.store.assignees).toHaveLength(1);
+    });
+
+    it('sets changing to true', () => {
+      expect(testContext.store.changing).toBe(true);
+    });
+  });
+
+  describe('when it removes an assignee', () => {
+    beforeEach(() => {
+      testContext.store.removeAssignee(ASSIGNEE);
+    });
+
+    it('removes an assignee', () => {
+      expect(testContext.store.assignees).toHaveLength(0);
+    });
+
+    it('sets changing to true', () => {
+      expect(testContext.store.changing).toBe(true);
+    });
   });
 
   it('finds an existent assignee', () => {
@@ -86,6 +110,7 @@ describe('Sidebar store', () => {
     testContext.store.removeAllAssignees();
 
     expect(testContext.store.assignees.length).toEqual(0);
+    expect(testContext.store.changing).toBe(true);
   });
 
   it('sets participants data', () => {

spec/models/application_record_spec.rb

@@ -90,4 +90,12 @@ RSpec.describe ApplicationRecord do
       expect(User.at_most(2).count).to eq(2)
     end
   end
+
+  describe '.where_exists' do
+    it 'produces a WHERE EXISTS query' do
+      user = create(:user)
+
+      expect(User.where_exists(User.limit(1))).to eq([user])
+    end
+  end
 end

spec/models/ci/build_trace_chunk_spec.rb

@@ -508,20 +508,6 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
 
     subject { build_trace_chunk.persist_data! }
 
-    shared_examples_for 'Atomic operation' do
-      context 'when the other process is persisting' do
-        let(:lease_key) { "trace_write:#{build_trace_chunk.build.id}:chunks:#{build_trace_chunk.chunk_index}" }
-
-        before do
-          stub_exclusive_lease_taken(lease_key)
-        end
-
-        it 'raise an error' do
-          expect { subject }.to raise_error('Failed to obtain a lock')
-        end
-      end
-    end
-
     context 'when data_store is redis' do
       let(:data_store) { :redis }
 
@@ -552,8 +538,6 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
 
             expect(build_trace_chunk.reload.checksum).to eq '3398914352'
           end
-
-          it_behaves_like 'Atomic operation'
         end
 
         context 'when data size has not reached CHUNK_SIZE' do
@@ -606,6 +590,35 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
                                 /Modifed build trace chunk detected/)
             end
           end
+
+          context 'when the chunk is being locked by a different worker' do
+            let(:metrics) { spy('metrics') }
+
+            it 'does not raise an exception' do
+              lock_chunk do
+                expect { build_trace_chunk.persist_data! }.not_to raise_error
+              end
+            end
+
+            it 'increments stalled chunk trace metric' do
+              allow(build_trace_chunk)
+                .to receive(:metrics)
+                .and_return(metrics)
+
+              lock_chunk { build_trace_chunk.persist_data! }
+
+              expect(metrics)
+                .to have_received(:increment_trace_operation)
+                .with(operation: :stalled)
+                .once
+            end
+
+            def lock_chunk(&block)
+              "trace_write:#{build.id}:chunks:#{chunk_index}".then do |key|
+                build_trace_chunk.in_lock(key, &block)
+              end
+            end
+          end
         end
       end
 
@@ -640,8 +653,6 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
             expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil
             expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data)
           end
-
-          it_behaves_like 'Atomic operation'
         end
 
         context 'when data size has not reached CHUNK_SIZE' do
@@ -701,8 +712,6 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
             expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil
             expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data)
           end
-
-          it_behaves_like 'Atomic operation'
         end
 
         context 'when data size has not reached CHUNK_SIZE' do

spec/support/shared_examples/features/multiple_assignees_mr_shared_examples.rb

@@ -38,7 +38,7 @@ RSpec.shared_examples 'multiple assignees merge request' do |action, save_button
     page.within '.issuable-sidebar' do
       page.within '.assignee' do
         # Closing dropdown to persist
-        click_link 'Edit'
+        click_link 'Apply'
 
         expect(page).to have_content user2.name
       end