From 75776cf237c2cc061eadcbabf8ef508eaec50d25 Mon Sep 17 00:00:00 2001 From: unarist Date: Fri, 27 Oct 2017 23:10:22 +0900 Subject: [PATCH 1/3] Optimize FixReblogsInFeeds migration (#5538) We have changed how we store reblogs in the redis for bigint IDs. This process is done by 1) scan all entries in users feed, and 2) re-store reblogs by 3 write commands. However, this operation is really slow for large instances. e.g. 1hrs on friends.nico (w/ 50k users). So I have tried below tweaks. * It checked non-reblogs by `entry[0] == entry[1]`, but this condition won't work because `entry[0]` is String while `entry[1]` is Float. Changing `entry[0].to_i == entry[1]` seems work. -> about 4-20x faster (feed with less reblogs will be faster) * Write operations can be batched by pipeline -> about 6x faster * Wrap operation by Lua script and execute by EVALSHA command. This really reduces packets between Ruby and Redis. -> about 3x faster I've taken Lua script way, though doing other optimizations may be enough. --- .../20170920032311_fix_reblogs_in_feeds.rb | 84 +++++++++++-------- 1 file changed, 49 insertions(+), 35 deletions(-) diff --git a/db/migrate/20170920032311_fix_reblogs_in_feeds.rb b/db/migrate/20170920032311_fix_reblogs_in_feeds.rb index c813ecd469..439c5fca08 100644 --- a/db/migrate/20170920032311_fix_reblogs_in_feeds.rb +++ b/db/migrate/20170920032311_fix_reblogs_in_feeds.rb @@ -3,48 +3,62 @@ class FixReblogsInFeeds < ActiveRecord::Migration[5.1] redis = Redis.current fm = FeedManager.instance - # find_each is batched on the database side. - User.includes(:account).find_each do |user| - account = user.account + # Old scheme: + # Each user's feed zset had a series of score:value entries, + # where "regular" statuses had the same score and value (their + # ID). Reblogs had a score of the reblogging status' ID, and a + # value of the reblogged status' ID. - # Old scheme: - # Each user's feed zset had a series of score:value entries, - # where "regular" statuses had the same score and value (their - # ID). Reblogs had a score of the reblogging status' ID, and a - # value of the reblogged status' ID. - - # New scheme: - # The feed contains only entries with the same score and value. - # Reblogs result in the reblogging status being added to the - # feed, with an entry in a reblog tracking zset (where the score - # is once again set to the reblogging status' ID, and the value - # is set to the reblogged status' ID). This is safe for Redis' - # float coersion because in this reblog tracking zset, we only - # need the rebloggging status' ID to be able to stop tracking - # entries after they have gotten too far down the feed, which - # does not require an exact value. - - # So, first, we iterate over the user's feed to find any reblogs. - timeline_key = fm.key(:home, account.id) - reblog_key = fm.key(:home, account.id, 'reblogs') - redis.zrange(timeline_key, 0, -1, with_scores: true).each do |entry| - next if entry[0] == entry[1] + # New scheme: + # The feed contains only entries with the same score and value. + # Reblogs result in the reblogging status being added to the + # feed, with an entry in a reblog tracking zset (where the score + # is once again set to the reblogging status' ID, and the value + # is set to the reblogged status' ID). This is safe for Redis' + # float coersion because in this reblog tracking zset, we only + # need the rebloggging status' ID to be able to stop tracking + # entries after they have gotten too far down the feed, which + # does not require an exact value. + + # This process reads all feeds and writes 3 times for each reblogs. + # So we use Lua script to avoid overhead between Ruby and Redis. + script = <<-LUA + local timeline_key = KEYS[1] + local reblog_key = KEYS[2] - # The score and value don't match, so this is a reblog. - # (note that we're transitioning from IDs < 53 bits so we - # don't have to worry about the loss of precision) + -- So, first, we iterate over the user's feed to find any reblogs. + local items = redis.call('zrange', timeline_key, 0, -1, 'withscores') + + for i = 1, #items, 2 do + local reblogged_id = items[i] + local reblogging_id = items[i + 1] + if (reblogged_id ~= reblogging_id) then - reblogged_id, reblogging_id = entry + -- The score and value don't match, so this is a reblog. + -- (note that we're transitioning from IDs < 53 bits so we + -- don't have to worry about the loss of precision) - # Remove the old entry - redis.zrem(timeline_key, reblogged_id) + -- Remove the old entry + redis.call('zrem', timeline_key, reblogged_id) - # Add a new one for the reblogging status - redis.zadd(timeline_key, reblogging_id, reblogging_id) + -- Add a new one for the reblogging status + redis.call('zadd', timeline_key, reblogging_id, reblogging_id) - # Track the fact that this was a reblog - redis.zadd(reblog_key, reblogging_id, reblogged_id) + -- Track the fact that this was a reblog + redis.call('zadd', reblog_key, reblogging_id, reblogged_id) + end end + LUA + script_hash = redis.script(:load, script) + + # find_each is batched on the database side. + User.includes(:account).find_each do |user| + account = user.account + + timeline_key = fm.key(:home, account.id) + reblog_key = fm.key(:home, account.id, 'reblogs') + + redis.evalsha(script_hash, [timeline_key, reblog_key]) end end From 2c06b218838ad2374939b1a75b0ac72c16a786aa Mon Sep 17 00:00:00 2001 From: puckipedia Date: Fri, 27 Oct 2017 16:10:36 +0200 Subject: [PATCH 2/3] Allow ActivityPub Note's tag and attachment to be single objects (#5534) --- app/helpers/jsonld_helper.rb | 4 ++++ app/lib/activitypub/activity/create.rb | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb index c23a2e0952..a3441e6f93 100644 --- a/app/helpers/jsonld_helper.rb +++ b/app/helpers/jsonld_helper.rb @@ -9,6 +9,10 @@ module JsonLdHelper value.is_a?(Array) ? value.first : value end + def as_array(value) + value.is_a?(Array) ? value : [value] + end + def value_or_id(value) value.is_a?(String) || value.nil? ? value : value['id'] end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index d6e9bc1def..376684c006 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -53,9 +53,9 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_tags(status) - return unless @object['tag'].is_a?(Array) + return if @object['tag'].nil? - @object['tag'].each do |tag| + as_array(@object['tag']).each do |tag| case tag['type'] when 'Hashtag' process_hashtag tag, status @@ -103,9 +103,9 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end def process_attachments(status) - return unless @object['attachment'].is_a?(Array) + return if @object['attachment'].nil? - @object['attachment'].each do |attachment| + as_array(@object['attachment']).each do |attachment| next if unsupported_media_type?(attachment['mediaType']) || attachment['url'].blank? href = Addressable::URI.parse(attachment['url']).normalize.to_s From 0a9cc96cdcd3750aa5e94f57312d2b008a12a863 Mon Sep 17 00:00:00 2001 From: nullkal Date: Fri, 27 Oct 2017 23:11:30 +0900 Subject: [PATCH 3/3] Feature: Unlisted custom emojis (#5485) --- app/controllers/admin/custom_emojis_controller.rb | 10 +++++++++- .../containers/emoji_picker_dropdown_container.js | 2 +- app/models/custom_emoji.rb | 1 + app/serializers/rest/custom_emoji_serializer.rb | 2 +- app/views/admin/custom_emojis/_custom_emoji.html.haml | 7 ++++++- config/locales/en.yml | 4 ++++ config/routes.rb | 2 +- ...1020084748_add_visible_in_picker_to_custom_emoji.rb | 7 +++++++ db/schema.rb | 3 ++- 9 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20171020084748_add_visible_in_picker_to_custom_emoji.rb diff --git a/app/controllers/admin/custom_emojis_controller.rb b/app/controllers/admin/custom_emojis_controller.rb index 5d9144be7b..cbd7abe958 100644 --- a/app/controllers/admin/custom_emojis_controller.rb +++ b/app/controllers/admin/custom_emojis_controller.rb @@ -22,6 +22,14 @@ module Admin end end + def update + if @custom_emoji.update(resource_params) + redirect_to admin_custom_emojis_path, notice: I18n.t('admin.custom_emojis.updated_msg') + else + redirect_to admin_custom_emojis_path, notice: I18n.t('admin.custom_emojis.update_failed_msg') + end + end + def destroy @custom_emoji.destroy redirect_to admin_custom_emojis_path, notice: I18n.t('admin.custom_emojis.destroyed_msg') @@ -56,7 +64,7 @@ module Admin end def resource_params - params.require(:custom_emoji).permit(:shortcode, :image) + params.require(:custom_emoji).permit(:shortcode, :image, :visible_in_picker) end def filtered_custom_emojis diff --git a/app/javascript/mastodon/features/compose/containers/emoji_picker_dropdown_container.js b/app/javascript/mastodon/features/compose/containers/emoji_picker_dropdown_container.js index 71944128cd..699687c698 100644 --- a/app/javascript/mastodon/features/compose/containers/emoji_picker_dropdown_container.js +++ b/app/javascript/mastodon/features/compose/containers/emoji_picker_dropdown_container.js @@ -46,7 +46,7 @@ const getFrequentlyUsedEmojis = createSelector([ const getCustomEmojis = createSelector([ state => state.get('custom_emojis'), -], emojis => emojis.sort((a, b) => { +], emojis => emojis.filter(e => e.get('visible_in_picker')).sort((a, b) => { const aShort = a.get('shortcode').toLowerCase(); const bShort = b.get('shortcode').toLowerCase(); diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb index 65d9840d56..28b6a2b0bc 100644 --- a/app/models/custom_emoji.rb +++ b/app/models/custom_emoji.rb @@ -15,6 +15,7 @@ # disabled :boolean default(FALSE), not null # uri :string # image_remote_url :string +# visible_in_picker :boolean default(TRUE), not null # class CustomEmoji < ApplicationRecord diff --git a/app/serializers/rest/custom_emoji_serializer.rb b/app/serializers/rest/custom_emoji_serializer.rb index b958e6a5db..65686a8666 100644 --- a/app/serializers/rest/custom_emoji_serializer.rb +++ b/app/serializers/rest/custom_emoji_serializer.rb @@ -3,7 +3,7 @@ class REST::CustomEmojiSerializer < ActiveModel::Serializer include RoutingHelper - attributes :shortcode, :url, :static_url + attributes :shortcode, :url, :static_url, :visible_in_picker def url full_asset_url(object.image.url) diff --git a/app/views/admin/custom_emojis/_custom_emoji.html.haml b/app/views/admin/custom_emojis/_custom_emoji.html.haml index 1fa64908c0..399d13bbd6 100644 --- a/app/views/admin/custom_emojis/_custom_emoji.html.haml +++ b/app/views/admin/custom_emojis/_custom_emoji.html.haml @@ -9,7 +9,12 @@ - else = custom_emoji.domain %td - - unless custom_emoji.local? + - if custom_emoji.local? + - if custom_emoji.visible_in_picker + = table_link_to 'eye', t('admin.custom_emojis.listed'), admin_custom_emoji_path(custom_emoji, custom_emoji: { visible_in_picker: false }), method: :patch + - else + = table_link_to 'eye-slash', t('admin.custom_emojis.unlisted'), admin_custom_emoji_path(custom_emoji, custom_emoji: { visible_in_picker: true }), method: :patch + - else = table_link_to 'copy', t('admin.custom_emojis.copy'), copy_admin_custom_emoji_path(custom_emoji, page: params[:page]), method: :post %td - if custom_emoji.disabled? diff --git a/config/locales/en.yml b/config/locales/en.yml index 45929e97dd..2d821550af 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -130,11 +130,15 @@ en: enable: Enable enabled_msg: Successfully enabled that emoji image_hint: PNG up to 50KB + listed: Listed new: title: Add new custom emoji shortcode: Shortcode shortcode_hint: At least 2 characters, only alphanumeric characters and underscores title: Custom emojis + unlisted: Unlisted + update_failed_msg: Could not update that emoji + updated_msg: Emoji successfully updated! upload: Upload domain_blocks: add_new: Add new diff --git a/config/routes.rb b/config/routes.rb index 5a6351f779..bdfcdaff69 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -140,7 +140,7 @@ Rails.application.routes.draw do resource :two_factor_authentication, only: [:destroy] end - resources :custom_emojis, only: [:index, :new, :create, :destroy] do + resources :custom_emojis, only: [:index, :new, :create, :update, :destroy] do member do post :copy post :enable diff --git a/db/migrate/20171020084748_add_visible_in_picker_to_custom_emoji.rb b/db/migrate/20171020084748_add_visible_in_picker_to_custom_emoji.rb new file mode 100644 index 0000000000..60a2871015 --- /dev/null +++ b/db/migrate/20171020084748_add_visible_in_picker_to_custom_emoji.rb @@ -0,0 +1,7 @@ +class AddVisibleInPickerToCustomEmoji < ActiveRecord::Migration[5.1] + def change + safety_assured { + add_column :custom_emojis, :visible_in_picker, :boolean, default: true, null: false + } + end +end diff --git a/db/schema.rb b/db/schema.rb index f9722ccda0..697a7f374b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171010025614) do +ActiveRecord::Schema.define(version: 20171020084748) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -111,6 +111,7 @@ ActiveRecord::Schema.define(version: 20171010025614) do t.boolean "disabled", default: false, null: false t.string "uri" t.string "image_remote_url" + t.boolean "visible_in_picker", default: true, null: false t.index ["shortcode", "domain"], name: "index_custom_emojis_on_shortcode_and_domain", unique: true end