Merge commit '501d6197c4' into kb_migration

app/javascript/mastodon/features/emoji/emoji.js

@@ -29,15 +29,15 @@ const emojifyTextNode = (node, customEmojis) => {
   let i = 0;
 
   for (;;) {
-    let match;
+    let unicode_emoji;
 
-    // Skip to the next potential emoji to replace
+    // Skip to the next potential emoji to replace (either custom emoji or custom emoji :shortcode:
     if (customEmojis === null) {
-      while (i < str.length && !(match = trie.search(str.slice(i)))) {
+      while (i < str.length && !(unicode_emoji = trie.search(str.slice(i)))) {
         i += str.codePointAt(i) < 65536 ? 1 : 2;
       }
     } else {
-      while (i < str.length && str[i] !== ':' && !(match = trie.search(str.slice(i)))) {
+      while (i < str.length && str[i] !== ':' && !(unicode_emoji = trie.search(str.slice(i)))) {
         i += str.codePointAt(i) < 65536 ? 1 : 2;
       }
     }
@@ -48,29 +48,37 @@ const emojifyTextNode = (node, customEmojis) => {
     }
 
     let rend, replacement = null;
-    if (str[i] === ':') { // Potentially the start of a custom emoji shortcode
-      if (!(rend = str.indexOf(':', i + 1) + 1)) {
-        continue; // no pair of ':'
+    if (str[i] === ':') { // Potentially the start of a custom emoji :shortcode:
+      rend = str.indexOf(':', i + 1) + 1;
+
+      // no matching ending ':', skip
+      if (!rend) {
+        i++;
+        continue;
       }
 
-      const shortname = str.slice(i, rend);
-      // now got a replacee as ':shortname:'
+      const shortcode = str.slice(i, rend);
+      const custom_emoji = customEmojis[shortcode];
+
+      // not a recognized shortcode, skip
+      if (!custom_emoji) {
+        i++;
+        continue;
+      }
+
+      // now got a replacee as ':shortcode:'
       // if you want additional emoji handler, add statements below which set replacement and return true.
-      if (shortname in customEmojis) {
-        const filename = autoPlayGif ? customEmojis[shortname].url : customEmojis[shortname].static_url;
+      const filename = autoPlayGif ? custom_emoji.url : custom_emoji.static_url;
       replacement = document.createElement('img');
       replacement.setAttribute('draggable', 'false');
       replacement.setAttribute('class', 'emojione custom-emoji');
-        replacement.setAttribute('alt', shortname);
-        replacement.setAttribute('title', shortname);
+      replacement.setAttribute('alt', shortcode);
+      replacement.setAttribute('title', shortcode);
       replacement.setAttribute('src', filename);
-        replacement.setAttribute('data-original', customEmojis[shortname].url);
-        replacement.setAttribute('data-static', customEmojis[shortname].static_url);
-      } else {
-        continue;
-      }
-    } else { // matched to unicode emoji
-      rend = i + match.length;
+      replacement.setAttribute('data-original', custom_emoji.url);
+      replacement.setAttribute('data-static', custom_emoji.static_url);
+    } else { // start of an unicode emoji
+      rend = i + unicode_emoji.length;
 
       // If the matched character was followed by VS15 (for selecting text presentation), skip it.
       if (str.codePointAt(rend - 1) !== VS16 && str.codePointAt(rend) === VS15) {
@@ -78,13 +86,13 @@ const emojifyTextNode = (node, customEmojis) => {
         continue;
       }
 
-      const { filename, shortCode } = unicodeMapping[match];
+      const { filename, shortCode } = unicodeMapping[unicode_emoji];
       const title = shortCode ? `:${shortCode}:` : '';
 
       replacement = document.createElement('img');
       replacement.setAttribute('draggable', 'false');
       replacement.setAttribute('class', 'emojione');
-      replacement.setAttribute('alt', match);
+      replacement.setAttribute('alt', unicode_emoji);
       replacement.setAttribute('title', title);
       replacement.setAttribute('src', `${assetHost}/emoji/${emojiFilename(filename)}.svg`);
     }

app/models/account_statuses_cleanup_policy.rb

@@ -57,9 +57,9 @@ class AccountStatusesCleanupPolicy < ApplicationRecord
   before_save :update_last_inspected
 
   def statuses_to_delete(limit = 50, max_id = nil, min_id = nil)
-    scope = account.statuses
+    scope = account_statuses
     scope.merge!(old_enough_scope(max_id))
-    scope = scope.where(Status.arel_table[:id].gteq(min_id)) if min_id.present?
+    scope = scope.where(id: min_id..) if min_id.present?
     scope.merge!(without_popular_scope) unless min_favs.nil? && min_reblogs.nil?
     scope.merge!(without_direct_scope) if keep_direct?
     scope.merge!(without_pinned_scope) if keep_pinned?
@@ -80,7 +80,7 @@ class AccountStatusesCleanupPolicy < ApplicationRecord
   def compute_cutoff_id
     min_id = last_inspected || 0
     max_id = Mastodon::Snowflake.id_at(min_status_age.seconds.ago, with_random: false)
-    subquery = account.statuses.where(Status.arel_table[:id].gteq(min_id)).where(Status.arel_table[:id].lteq(max_id))
+    subquery = account_statuses.where(id: min_id..max_id)
     subquery = subquery.select(:id).reorder(id: :asc).limit(EARLY_SEARCH_CUTOFF)
 
     # We're textually interpolating a subquery here as ActiveRecord seem to not provide
@@ -91,11 +91,11 @@ class AccountStatusesCleanupPolicy < ApplicationRecord
   # The most important thing about `last_inspected` is that any toot older than it is guaranteed
   # not to be kept by the policy regardless of its age.
   def record_last_inspected(last_id)
-    redis.set("account_cleanup:#{account.id}", last_id, ex: 1.week.seconds)
+    redis.set("account_cleanup:#{account_id}", last_id, ex: 2.weeks.seconds)
   end
 
   def last_inspected
-    redis.get("account_cleanup:#{account.id}")&.to_i
+    redis.get("account_cleanup:#{account_id}")&.to_i
   end
 
   def invalidate_last_inspected(status, action)
@@ -120,9 +120,9 @@ class AccountStatusesCleanupPolicy < ApplicationRecord
     if EXCEPTION_BOOLS.map { |name| attribute_change_to_be_saved(name) }.compact.include?([true, false])
       # Policy has been widened in such a way that any previously-inspected status
       # may need to be deleted, so we'll have to start again.
-      redis.del("account_cleanup:#{account.id}")
+      redis.del("account_cleanup:#{account_id}")
     end
-    redis.del("account_cleanup:#{account.id}") if EXCEPTION_THRESHOLDS.map { |name| attribute_change_to_be_saved(name) }.compact.any? { |old, new| old.present? && (new.nil? || new > old) }
+    redis.del("account_cleanup:#{account_id}") if EXCEPTION_THRESHOLDS.map { |name| attribute_change_to_be_saved(name) }.compact.any? { |old, new| old.present? && (new.nil? || new > old) }
   end
 
   def validate_local_account
@@ -141,23 +141,23 @@ class AccountStatusesCleanupPolicy < ApplicationRecord
 
     max_id = snowflake_id if max_id.nil? || snowflake_id < max_id
 
-    Status.where(Status.arel_table[:id].lteq(max_id))
+    Status.where(id: ..max_id)
   end
 
   def without_self_fav_scope
-    Status.where('NOT EXISTS (SELECT * FROM favourites fav WHERE fav.account_id = statuses.account_id AND fav.status_id = statuses.id)')
+    Status.where('NOT EXISTS (SELECT 1 FROM favourites fav WHERE fav.account_id = statuses.account_id AND fav.status_id = statuses.id)')
   end
 
   def without_self_bookmark_scope
-    Status.where('NOT EXISTS (SELECT * FROM bookmarks bookmark WHERE bookmark.account_id = statuses.account_id AND bookmark.status_id = statuses.id)')
+    Status.where('NOT EXISTS (SELECT 1 FROM bookmarks bookmark WHERE bookmark.account_id = statuses.account_id AND bookmark.status_id = statuses.id)')
   end
 
   def without_pinned_scope
-    Status.where('NOT EXISTS (SELECT * FROM status_pins pin WHERE pin.account_id = statuses.account_id AND pin.status_id = statuses.id)')
+    Status.where('NOT EXISTS (SELECT 1 FROM status_pins pin WHERE pin.account_id = statuses.account_id AND pin.status_id = statuses.id)')
   end
 
   def without_media_scope
-    Status.where('NOT EXISTS (SELECT * FROM media_attachments media WHERE media.status_id = statuses.id)')
+    Status.where('NOT EXISTS (SELECT 1 FROM media_attachments media WHERE media.status_id = statuses.id)')
   end
 
   def without_poll_scope
@@ -170,4 +170,8 @@ class AccountStatusesCleanupPolicy < ApplicationRecord
     scope = scope.where('COALESCE(status_stats.favourites_count, 0) < ?', min_favs) unless min_favs.nil?
     scope
   end
+
+  def account_statuses
+    Status.where(account_id: account_id)
+  end
 end

app/workers/scheduler/accounts_statuses_cleanup_scheduler.rb

@@ -7,28 +7,30 @@ class Scheduler::AccountsStatusesCleanupScheduler
   # This limit is mostly to be nice to the fediverse at large and not
   # generate too much traffic.
   # This also helps limiting the running time of the scheduler itself.
-  MAX_BUDGET         = 150
+  MAX_BUDGET         = 300
 
-  # This is an attempt to spread the load across instances, as various
-  # accounts are likely to have various followers.
+  # This is an attempt to spread the load across remote servers, as
+  # spreading deletions across diverse accounts is likely to spread
+  # the deletion across diverse followers. It also helps each individual
+  # user see some effect sooner.
   PER_ACCOUNT_BUDGET = 5
 
   # This is an attempt to limit the workload generated by status removal
-  # jobs to something the particular instance can handle.
-  PER_THREAD_BUDGET  = 6
+  # jobs to something the particular server can handle.
+  PER_THREAD_BUDGET  = 5
 
-  # Those avoid loading an instance that is already under load
-  MAX_DEFAULT_SIZE    = 200
-  MAX_DEFAULT_LATENCY = 5
-  MAX_PUSH_SIZE       = 500
-  MAX_PUSH_LATENCY    = 10
-
-  # 'pull' queue has lower priority jobs, and it's unlikely that pushing
-  # deletes would cause much issues with this queue if it didn't cause issues
-  # with default and push. Yet, do not enqueue deletes if the instance is
-  # lagging behind too much.
-  MAX_PULL_SIZE       = 10_000
-  MAX_PULL_LATENCY    = 5.minutes.to_i
+  # These are latency limits on various queues above which a server is
+  # considered to be under load, causing the auto-deletion to be entirely
+  # skipped for that run.
+  LOAD_LATENCY_THRESHOLDS = {
+    default: 5,
+    push: 10,
+    # The `pull` queue has lower priority jobs, and it's unlikely that
+    # pushing deletes would cause much issues with this queue if it didn't
+    # cause issues with `default` and `push`. Yet, do not enqueue deletes
+    # if the instance is lagging behind too much.
+    pull: 5.minutes.to_i,
+  }.freeze
 
   sidekiq_options retry: 0, lock: :until_executed, lock_ttl: 1.day.to_i
 
@@ -62,19 +64,20 @@ class Scheduler::AccountsStatusesCleanupScheduler
   end
 
   def compute_budget
+    # Each post deletion is a `RemovalWorker` job (on `default` queue), each
+    # potentially spawning many `ActivityPub::DeliveryWorker` jobs (on the `push` queue).
     threads = Sidekiq::ProcessSet.new.select { |x| x['queues'].include?('push') }.pluck('concurrency').sum
     [PER_THREAD_BUDGET * threads, MAX_BUDGET].min
   end
 
   def under_load?
-    queue_under_load?('default', MAX_DEFAULT_SIZE, MAX_DEFAULT_LATENCY) || queue_under_load?('push', MAX_PUSH_SIZE, MAX_PUSH_LATENCY) || queue_under_load?('pull', MAX_PULL_SIZE, MAX_PULL_LATENCY)
+    LOAD_LATENCY_THRESHOLDS.any? { |queue, max_latency| queue_under_load?(queue, max_latency) }
   end
 
   private
 
-  def queue_under_load?(name, max_size, max_latency)
-    queue = Sidekiq::Queue.new(name)
-    queue.size > max_size || queue.latency > max_latency
+  def queue_under_load?(name, max_latency)
+    Sidekiq::Queue.new(name).latency > max_latency
   end
 
   def last_processed_id