From 87dd2dca604807257e25c580e5a05e4eb5d5d50a Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 7 May 2015 18:47:03 +0200 Subject: [PATCH 1/4] Add SIDEKIQ_MEMORY_KILLER_SHUTDOWN_SIGNAL env var It looks like SIGTERM may not be enough to shut down a Sidekiq process when its RSS has gotten too big. This change will allow us to experiment with sending SIGKILL instead of SIGTERM to Sidekiq processes on gitlab.com. --- CHANGELOG | 4 ++++ doc/operations/sidekiq_memory_killer.md | 2 ++ lib/gitlab/sidekiq_middleware/memory_killer.rb | 11 ++++++----- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2eec1d6a50..0960a96a6f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -43,6 +43,10 @@ v 7.10.0 (unreleased) - Prevent sending empty messages to HipChat (Chulki Lee) - Improve UI for mobile phones on dashboard and project pages - Add room notification and message color option for HipChat + - Allow to use non-ASCII letters and dashes in project and namespace name. (Jakub Jirutka) + - Add footnotes support to Markdown (Guillaume Delbergue) + - Add current_sign_in_at to UserFull REST api. + - Make Sidekiq MemoryKiller shutdown signal configurable v 7.10.2 - Fix CI links on MR page diff --git a/doc/operations/sidekiq_memory_killer.md b/doc/operations/sidekiq_memory_killer.md index 867b01b0d5..811c2192a1 100644 --- a/doc/operations/sidekiq_memory_killer.md +++ b/doc/operations/sidekiq_memory_killer.md @@ -36,3 +36,5 @@ The MemoryKiller is controlled using environment variables. Existing jobs get 30 seconds to finish. After that, the MemoryKiller tells Sidekiq to shut down, and an external supervision mechanism (e.g. Runit) must restart Sidekiq. +- `SIDEKIQ_MEMORY_KILLER_SHUTDOWN_SIGNAL`: defaults to 'SIGTERM'. The name of + the final signal sent to the Sidekiq process when we want it to shut down. diff --git a/lib/gitlab/sidekiq_middleware/memory_killer.rb b/lib/gitlab/sidekiq_middleware/memory_killer.rb index 0f2db50e98..f33b2dedf4 100644 --- a/lib/gitlab/sidekiq_middleware/memory_killer.rb +++ b/lib/gitlab/sidekiq_middleware/memory_killer.rb @@ -7,6 +7,7 @@ module Gitlab GRACE_TIME = (ENV['SIDEKIQ_MEMORY_KILLER_GRACE_TIME'] || 15 * 60).to_s.to_i # Wait 30 seconds for running jobs to finish during graceful shutdown SHUTDOWN_WAIT = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_WAIT'] || 30).to_s.to_i + SHUTDOWN_SIGNAL = (ENV['SIDEKIQ_MEMORY_KILLER_SHUTDOWN_SIGNAL'] || 'SIGTERM').to_s # Create a mutex used to ensure there will be only one thread waiting to # shut Sidekiq down @@ -24,19 +25,19 @@ module Gitlab Sidekiq.logger.warn "current RSS #{current_rss} exceeds maximum RSS "\ "#{MAX_RSS}" - Sidekiq.logger.warn "spawned thread that will shut down PID "\ - "#{Process.pid} in #{GRACE_TIME} seconds" + Sidekiq.logger.warn "this thread will shut down PID #{Process.pid} "\ + "in #{GRACE_TIME} seconds" sleep(GRACE_TIME) Sidekiq.logger.warn "sending SIGUSR1 to PID #{Process.pid}" Process.kill('SIGUSR1', Process.pid) Sidekiq.logger.warn "waiting #{SHUTDOWN_WAIT} seconds before sending "\ - "SIGTERM to PID #{Process.pid}" + "#{SHUTDOWN_SIGNAL} to PID #{Process.pid}" sleep(SHUTDOWN_WAIT) - Sidekiq.logger.warn "sending SIGTERM to PID #{Process.pid}" - Process.kill('SIGTERM', Process.pid) + Sidekiq.logger.warn "sending #{SHUTDOWN_SIGNAL} to PID #{Process.pid}" + Process.kill(SHUTDOWN_SIGNAL, Process.pid) end end From 708b507328a8efefeef71b108f9067e822896587 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Fri, 8 May 2015 08:30:05 +0000 Subject: [PATCH 2/4] Merge branch 'tag_dup' into 'master' Remove tag duplicates Prevention of getting this error: ``` ActiveRecord::RecordNotUnique: PG::Error: ERROR: could not create unique index "index_tags_on_name" DETAIL: Key (name)=(XSS') is duplicated. : CREATE UNIQUE INDEX "index_tags_on_name" ON "tags" ("name") ``` This migration fails if there are duplicates: https://github.com/mbleigh/acts-as-taggable-on/blob/v3.5.0/db/migrate/2_add_missing_unique_indices.rb#L3 See merge request !623 --- .../20150425164647_remove_duplicate_tags.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 db/migrate/20150425164647_remove_duplicate_tags.rb diff --git a/db/migrate/20150425164647_remove_duplicate_tags.rb b/db/migrate/20150425164647_remove_duplicate_tags.rb new file mode 100644 index 0000000000..1a9152cb96 --- /dev/null +++ b/db/migrate/20150425164647_remove_duplicate_tags.rb @@ -0,0 +1,16 @@ +class RemoveDuplicateTags < ActiveRecord::Migration + def up + select_all("SELECT name, COUNT(id) as cnt FROM tags GROUP BY name HAVING COUNT(id) > 1").each do |tag| + duplicate_ids = select_all("SELECT id FROM tags WHERE name = '#{tag["name"]}'").map{|tag| tag["id"]} + origin_tag_id = duplicate_ids.first + duplicate_ids.delete origin_tag_id + + execute("UPDATE taggings SET tag_id = #{origin_tag_id} WHERE tag_id IN(#{duplicate_ids.join(",")})") + execute("DELETE FROM tags WHERE id IN(#{duplicate_ids.join(",")})") + end + end + + def down + + end +end From 999761284de055ec9b48a9d0c02ad019b76cf83d Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Fri, 8 May 2015 08:27:38 +0000 Subject: [PATCH 3/4] Merge branch 'conditional-delete-taggings-indices' into 'master' Conditionally remove indices that may not exist in pre-GitLab v6.7 installations In pre-GitLab v6.7 installations, the `tagging` indices were directly added to the init schema and not added in a migration. Anyone using this older DB schema may not have these indicies, so only remove them if they are there. Closes #1593 See merge request !626 --- ...ssing_unique_indices.acts_as_taggable_on_engine.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/db/migrate/20150425164648_add_missing_unique_indices.acts_as_taggable_on_engine.rb b/db/migrate/20150425164648_add_missing_unique_indices.acts_as_taggable_on_engine.rb index 4ca676f6c7..c1b7868151 100644 --- a/db/migrate/20150425164648_add_missing_unique_indices.acts_as_taggable_on_engine.rb +++ b/db/migrate/20150425164648_add_missing_unique_indices.acts_as_taggable_on_engine.rb @@ -3,8 +3,15 @@ class AddMissingUniqueIndices < ActiveRecord::Migration def self.up add_index :tags, :name, unique: true - remove_index :taggings, :tag_id - remove_index :taggings, [:taggable_id, :taggable_type, :context] + # pre-GitLab v6.7.0 may not have these indices since there were no + # migrations for them + if index_exists?(:taggings, :tag_id) + remove_index :taggings, :tag_id + end + + if index_exists?(:taggings, [:taggable_id, :taggable_type, :context]) + remove_index :taggings, [:taggable_id, :taggable_type, :context] + end add_index :taggings, [:tag_id, :taggable_id, :taggable_type, :context, :tagger_id, :tagger_type], unique: true, name: 'taggings_idx' From 7880589da3a75ad3bf36d556307d265af7635732 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Fri, 8 May 2015 08:29:07 +0000 Subject: [PATCH 4/4] Merge branch 'fix-mysql-migration-with-tags' into 'master' Change the MySQL collation type to utf8_bin to ensure case-sensitive tags are supported MySQL tables had a collation type of utf8_ci, which makes the name case-insensitive. Previously this migration was run only after indices were built, but it needs to happen before if there is existing data that is case-sensitive. This is an idempotent change, so applying it again in the existing migration (20150425164651) should not change anything. This is related to !623, but this migration should run before that one. * Closes #1589 * Closes https://github.com/gitlabhq/gitlabhq/issues/9255 See merge request !625 --- ...llation_for_tag_names.acts_as_taggable_on_engine.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 db/migrate/20150425164646_gitlab_change_collation_for_tag_names.acts_as_taggable_on_engine.rb diff --git a/db/migrate/20150425164646_gitlab_change_collation_for_tag_names.acts_as_taggable_on_engine.rb b/db/migrate/20150425164646_gitlab_change_collation_for_tag_names.acts_as_taggable_on_engine.rb new file mode 100644 index 0000000000..281c88d2a7 --- /dev/null +++ b/db/migrate/20150425164646_gitlab_change_collation_for_tag_names.acts_as_taggable_on_engine.rb @@ -0,0 +1,10 @@ +# This migration is a duplicate of 20150425164651_change_collation_for_tag_names.acts_as_taggable_on_engine.rb +# It shold be applied before the index additions to ensure that `name` is case sensitive. + +class GitlabChangeCollationForTagNames < ActiveRecord::Migration + def up + if ActsAsTaggableOn::Utils.using_mysql? + execute("ALTER TABLE tags MODIFY name varchar(255) CHARACTER SET utf8 COLLATE utf8_bin;") + end + end +end