Merge branch...

Merge branch '27790-periodically-save-last-activity-date-data-from-redis-to-the-database-hscan' into 'master'

[Improved] Periodically save last activity date data from Redis to the database

See merge request !1597

app/models/user.rb

@@ -972,10 +972,6 @@ class User < ActiveRecord::Base
     end
   end
 
-  def record_activity
-    Gitlab::UserActivities::ActivitySet.record(self)
-  end
-
   def access_level
     if admin?
       :admin

app/services/users/activity_service.rb

@@ -14,7 +14,7 @@ module Users
     private
 
     def record_activity
-      @author.record_activity unless Gitlab::Geo.secondary?
+      Gitlab::UserActivities.record(@author.id) unless Gitlab::Geo.secondary?
 
       Rails.logger.debug("Recorded activity: #{@activity} for User ID: #{@author.id} (username: #{@author.username}")
     end

app/workers/schedule_update_user_activity_worker.rb

+class ScheduleUpdateUserActivityWorker
+  include Sidekiq::Worker
+  include CronjobQueue
+
+  def perform(batch_size = 500)
+    return if Gitlab::Geo.secondary?
+
+    Gitlab::UserActivities.new.each_slice(batch_size) do |batch|
+      UpdateUserActivityWorker.perform_async(Hash[batch])
+    end
+  end
+end

app/workers/update_user_activity_worker.rb

+class UpdateUserActivityWorker
+  include Sidekiq::Worker
+  include DedicatedSidekiqQueue
+
+  def perform(pairs)
+    return if Gitlab::Geo.secondary?
+
+    pairs = cast_data(pairs)
+    ids = pairs.keys
+    conditions = 'WHEN id = ? THEN ? ' * ids.length
+
+    User.where(id: ids).
+      update_all([
+        "last_activity_on = CASE #{conditions} ELSE last_activity_on END",
+        *pairs.to_a.flatten
+      ])
+
+    Gitlab::UserActivities.new.delete(*ids)
+  end
+
+  private
+
+  def cast_data(pairs)
+    pairs.each_with_object({}) do |(key, value), new_pairs|
+      new_pairs[key.to_i] = Time.at(value.to_i).to_s(:db)
+    end
+  end
+end

changelogs/unreleased-ee/27790-periodically-save-last-activity-date-data-from-redis-to-the-database.yml

+---
+title: Periodically persists users activity to users.last_activity_on
+merge_request: 1597
+author:

config/initializers/1_settings.rb

@@ -432,6 +432,11 @@ Settings.cron_jobs['clear_shared_runners_minutes_worker'] ||= Settingslogic.new(
 Settings.cron_jobs['clear_shared_runners_minutes_worker']['cron'] ||= '0 0 1 * *'
 Settings.cron_jobs['clear_shared_runners_minutes_worker']['job_class'] = 'ClearSharedRunnersMinutesWorker'
 
+# Every day at 00:30
+Settings.cron_jobs['schedule_update_user_activity_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['schedule_update_user_activity_worker']['cron'] ||= '30 0 * * *'
+Settings.cron_jobs['schedule_update_user_activity_worker']['job_class'] = 'ScheduleUpdateUserActivityWorker'
+
 #
 # GitLab Shell
 #

config/sidekiq_queues.yml

@@ -63,3 +63,4 @@
   - [elastic_indexer, 1]
   - [elastic_commit_indexer, 1]
   - [export_csv, 1]
+  - [update_user_activity, 1]

db/migrate/20170307125949_add_last_activity_on_to_users.rb

+class AddLastActivityOnToUsers < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :users, :last_activity_on, :date
+  end
+end

db/post_migrate/20170324160416_migrate_user_activities_to_users_last_activity_on.rb

+class MigrateUserActivitiesToUsersLastActivityOn < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+  USER_ACTIVITY_SET_KEY = 'user/activities'.freeze
+  ACTIVITIES_PER_PAGE = 100
+  TIME_WHEN_ACTIVITY_SET_WAS_INTRODUCED = Time.utc(2016, 12, 1)
+
+  def up
+    return if activities_count(TIME_WHEN_ACTIVITY_SET_WAS_INTRODUCED, Time.now).zero?
+
+    day = Time.at(activities(TIME_WHEN_ACTIVITY_SET_WAS_INTRODUCED, Time.now).first.second)
+
+    transaction do
+      while day <= Time.now.utc.tomorrow
+        persist_last_activity_on(day: day)
+        day = day.tomorrow
+      end
+    end
+  end
+
+  def down
+    # This ensures we don't lock all users for the duration of the migration.
+    update_column_in_batches(:users, :last_activity_on, nil) do |table, query|
+      query.where(table[:last_activity_on].not_eq(nil))
+    end
+  end
+
+  private
+
+  def persist_last_activity_on(day:, page: 1)
+    activities_count = activities_count(day.at_beginning_of_day, day.at_end_of_day)
+
+    return if activities_count.zero?
+
+    activities = activities(day.at_beginning_of_day, day.at_end_of_day, page: page)
+
+    update_sql =
+      Arel::UpdateManager.new(ActiveRecord::Base).
+        table(users_table).
+        set(users_table[:last_activity_on] => day.to_date).
+        where(users_table[:username].in(activities.map(&:first))).
+        to_sql
+
+    connection.exec_update(update_sql, self.class.name, [])
+
+    unless last_page?(page, activities_count)
+      persist_last_activity_on(day: day, page: page + 1)
+    end
+  end
+
+  def users_table
+    @users_table ||= Arel::Table.new(:users)
+  end
+
+  def activities(from, to, page: 1)
+    Gitlab::Redis.with do |redis|
+      redis.zrangebyscore(USER_ACTIVITY_SET_KEY, from.to_i, to.to_i,
+        with_scores: true,
+        limit: limit(page))
+    end
+  end
+
+  def activities_count(from, to)
+    Gitlab::Redis.with do |redis|
+      redis.zcount(USER_ACTIVITY_SET_KEY, from.to_i, to.to_i)
+    end
+  end
+
+  def limit(page)
+    [offset(page), ACTIVITIES_PER_PAGE]
+  end
+
+  def total_pages(count)
+    (count.to_f / ACTIVITIES_PER_PAGE).ceil
+  end
+
+  def last_page?(page, count)
+    page >= total_pages(count)
+  end
+
+  def offset(page)
+    (page - 1) * ACTIVITIES_PER_PAGE
+  end
+end

db/schema.rb

@@ -1483,6 +1483,7 @@ ActiveRecord::Schema.define(version: 20170405080720) do
     t.boolean "authorized_projects_populated"
     t.boolean "auditor", default: false, null: false
     t.boolean "ghost"
+    t.date "last_activity_on"
     t.boolean "notified_of_own_activity"
     t.boolean "support_bot"
   end

doc/api/users.md

@@ -74,6 +74,7 @@ GET /users
     "organization": "",
     "last_sign_in_at": "2012-06-01T11:41:01Z",
     "confirmed_at": "2012-05-23T09:05:22Z",
+    "last_activity_on": "2012-05-23",
     "color_scheme_id": 2,
     "projects_limit": 100,
     "current_sign_in_at": "2012-06-02T06:36:55Z",
@@ -106,6 +107,7 @@ GET /users
     "organization": "",
     "last_sign_in_at": null,
     "confirmed_at": "2012-05-30T16:53:06.148Z",
+    "last_activity_on": "2012-05-23",
     "color_scheme_id": 3,
     "projects_limit": 100,
     "current_sign_in_at": "2014-03-19T17:54:13Z",
@@ -198,6 +200,7 @@ Parameters:
   "organization": "",
   "last_sign_in_at": "2012-06-01T11:41:01Z",
   "confirmed_at": "2012-05-23T09:05:22Z",
+  "last_activity_on": "2012-05-23",
   "color_scheme_id": 2,
   "projects_limit": 100,
   "current_sign_in_at": "2012-06-02T06:36:55Z",
@@ -322,6 +325,7 @@ GET /user
   "organization": "",
   "last_sign_in_at": "2012-06-01T11:41:01Z",
   "confirmed_at": "2012-05-23T09:05:22Z",
+  "last_activity_on": "2012-05-23",
   "color_scheme_id": 2,
   "projects_limit": 100,
   "current_sign_in_at": "2012-06-02T06:36:55Z",
@@ -367,6 +371,7 @@ GET /user
   "organization": "",
   "last_sign_in_at": "2012-06-01T11:41:01Z",
   "confirmed_at": "2012-05-23T09:05:22Z",
+  "last_activity_on": "2012-05-23",
   "color_scheme_id": 2,
   "projects_limit": 100,
   "current_sign_in_at": "2012-06-02T06:36:55Z",
@@ -999,16 +1004,9 @@ The activities that update the timestamp are:
   - Git HTTP/SSH activities (such as clone, push)
   - User logging in into GitLab
 
-The data is stored in Redis and it depends on it for being recorded and displayed
-over time. This means that we will lose the data if Redis gets flushed, or a custom
-TTL is reached.
-
 By default, it shows the activity for all users in the last 6 months, but this can be
 amended by using the `from` parameter.
 
-This function takes pagination parameters `page` and `per_page` to restrict the list of users.
-
-
 ```
 GET /user/activities
 ```
@@ -1029,15 +1027,20 @@ Example response:
 [
   {
     "username": "user1",
-    "last_activity_at": "2015-12-14 01:00:00"
+    "last_activity_on": "2015-12-14",
+    "last_activity_at": "2015-12-14"
   },
   {
     "username": "user2",
-    "last_activity_at": "2015-12-15 01:00:00"
+    "last_activity_on": "2015-12-15",
+    "last_activity_at": "2015-12-15"
   },
   {
     "username": "user3",
-    "last_activity_at": "2015-12-16 01:00:00"
+    "last_activity_on": "2015-12-16",
+    "last_activity_at": "2015-12-16"
   }
 ]
 ```
+
+Please note that `last_activity_at` is deprecated, please use `last_activity_on`.

lib/api/entities.rb

@@ -20,7 +20,8 @@ module API
 
     class UserActivity < Grape::Entity
       expose :username
-      expose :last_activity_at
+      expose :last_activity_on
+      expose :last_activity_on, as: :last_activity_at # Back-compat
     end
 
     class Identity < Grape::Entity
@@ -30,6 +31,7 @@ module API
     class UserPublic < User
       expose :last_sign_in_at
       expose :confirmed_at
+      expose :last_activity_on
       expose :email
       expose :color_scheme_id, :projects_limit, :current_sign_in_at
       expose :identities, using: Entities::Identity

lib/api/users.rb

@@ -537,19 +537,17 @@ module API
 
       desc 'Get a list of user activities'
       params do
-        optional :from, type: String, desc: 'Date string in the format YEAR-MONTH-DAY'
+        optional :from, type: DateTime, default: 6.months.ago, desc: 'Date string in the format YEAR-MONTH-DAY'
         use :pagination
       end
-      get ":activities" do
+      get "activities" do
         authenticated_as_admin!
 
-        activity_set = Gitlab::UserActivities::ActivitySet.new(from: params[:from],
-                                                               page: params[:page],
-                                                               per_page: params[:per_page])
+        activities = User.
+          where(User.arel_table[:last_activity_on].gteq(params[:from])).
+          reorder(last_activity_on: :asc)
 
-        add_pagination_headers(activity_set)
-
-        present activity_set.activities, with: Entities::UserActivity
+        present paginate(activities), with: Entities::UserActivity
       end
     end
   end

lib/gitlab/user_activities.rb

+module Gitlab
+  class UserActivities
+    include Enumerable
+
+    KEY = 'users:activities'.freeze
+    BATCH_SIZE = 500
+
+    def self.record(key, time = Time.now)
+      Gitlab::Redis.with do |redis|
+        redis.hset(KEY, key, time.to_i)
+      end
+    end
+
+    def delete(*keys)
+      Gitlab::Redis.with do |redis|
+        redis.hdel(KEY, keys)
+      end
+    end
+
+    def each
+      cursor = 0
+      loop do
+        cursor, pairs =
+          Gitlab::Redis.with do |redis|
+            redis.hscan(KEY, cursor, count: BATCH_SIZE)
+          end
+
+        Hash[pairs].each { |pair| yield pair }
+
+        break if cursor == '0'
+      end
+    end
+  end
+end

lib/gitlab/user_activities/activity.rb

-module Gitlab
-  module UserActivities
-    class Activity
-      attr_reader :username
-
-      def initialize(username, time)
-        @username = username
-        @time = time
-      end
-
-      def last_activity_at
-        @last_activity_at ||= Time.at(@time).to_s(:db)
-      end
-    end
-  end
-end

lib/gitlab/user_activities/activity_set.rb

-module Gitlab
-  module UserActivities
-    class ActivitySet
-      delegate :total_count,
-               :total_pages,
-               :current_page,
-               :limit_value,
-               :first_page?,
-               :prev_page,
-               :last_page?,
-               :next_page, to: :pagination_delegate
-
-      KEY = 'user/activities'.freeze
-
-      def self.record(user)
-        Gitlab::Redis.with do |redis|
-          redis.zadd(KEY, Time.now.to_i, user.username)
-        end
-      end
-
-      def initialize(from: nil, page: nil, per_page: nil)
-        @from = sanitize_date(from)
-        @to = Time.now.to_i
-        @page = page
-        @per_page = per_page
-      end
-
-      def activities
-        @activities ||= raw_activities.map { |activity| Activity.new(*activity) }
-      end
-
-      private
-
-      def sanitize_date(date)
-        Time.strptime(date, "%Y-%m-%d").to_i
-      rescue TypeError, ArgumentError
-        default_from
-      end
-
-      def pagination_delegate
-        @pagination_delegate ||= Gitlab::PaginationDelegate.new(page: @page,
-                                                                per_page: @per_page,
-                                                                count: count)
-      end
-
-      def raw_activities
-        Gitlab::Redis.with do |redis|
-          redis.zrangebyscore(KEY, @from, @to, with_scores: true, limit: limit)
-        end
-      end
-
-      def count
-        Gitlab::Redis.with do |redis|
-          redis.zcount(KEY, @from, @to)
-        end
-      end
-
-      def limit
-        [pagination_delegate.offset, pagination_delegate.limit_value]
-      end
-
-      def default_from
-        6.months.ago.to_i
-      end
-    end
-  end
-end

spec/controllers/sessions_controller_spec.rb

@@ -43,7 +43,7 @@ describe SessionsController do
         it 'updates the user activity' do
           expect do
             post(:create, user: { login: user.username, password: user.password })
-          end.to change { user_score }.from(0)
+          end.to change { user_activity(user) }
         end
       end
     end

spec/lib/gitlab/user_activities/activity_set_spec.rb

-require 'spec_helper'
-
-describe Gitlab::UserActivities::ActivitySet, :redis, lib: true do
-  let(:user) { create(:user) }
-
-  it 'shows the last user activity' do
-    Timecop.freeze do
-      user.record_activity
-
-      expect(described_class.new.activities.first).to be_an_instance_of(Gitlab::UserActivities::Activity)
-    end
-  end
-
-  context 'pagination delegation' do
-    let(:pagination_delegate) do
-      Gitlab::PaginationDelegate.new(page: 1,
-                                     per_page: 10,
-                                     count: 20)
-    end
-
-    let(:delegated_methods) { %i[total_count total_pages current_page limit_value first_page? prev_page last_page? next_page] }
-
-    before do
-      allow(described_class.new).to receive(:pagination_delegate).and_return(pagination_delegate)
-    end
-
-    it 'includes the delegated methods' do
-      expect(described_class.new.public_methods).to include(*delegated_methods)
-    end
-  end
-
-  context 'paginated activities' do
-    before do
-      Timecop.scale(3600)
-
-      7.times do
-        create(:user).record_activity
-      end
-    end
-
-    after do
-      Timecop.return
-    end
-
-    it 'shows the 5 oldest user activities paginated' do
-      expect(described_class.new(per_page: 5).activities.count).to eq(5)
-    end
-
-    it 'shows the 2 reamining user activities paginated' do
-      expect(described_class.new(per_page: 5, page: 2).activities.count).to eq(2)
-    end
-
-    it 'shows the oldest first' do
-      activities = described_class.new.activities
-
-      expect(activities.first.last_activity_at).to be < activities.last.last_activity_at
-    end
-  end
-
-  context 'filter by date' do
-    before do
-      create(:user).record_activity
-    end
-
-    it 'shows activities from today' do
-      today = Date.today.to_s("%Y-%m-%d")
-
-      expect(described_class.new(from: today).activities.count).to eq(1)
-    end
-
-    it 'filter activities from tomorrow' do
-      tomorrow = Date.tomorrow.to_s("%Y-%m-%d")
-
-      expect(described_class.new(from: tomorrow).activities.count).to eq(0)
-    end
-  end
-end

spec/lib/gitlab/user_activities/activity_spec.rb

-require 'spec_helper'
-
-describe Gitlab::UserActivities::Activity, :redis, lib: true do
-  let(:username) { 'user' }
-  let(:activity) { described_class.new('user', Time.new(2016, 12, 12).to_i) }
-
-  it 'has the username' do
-    expect(activity.username).to eq(username)
-  end
-
-  it 'has the last activity at' do
-    expect(activity.last_activity_at).to eq('2016-12-12 00:00:00')
-  end
-end

spec/lib/gitlab/user_activities_spec.rb

+require 'spec_helper'
+
+describe Gitlab::UserActivities, :redis, lib: true do
+  let(:now) { Time.now }
+
+  describe '.record' do
+    context 'with no time given' do
+      it 'uses Time.now and records an activity in Redis' do
+        Timecop.freeze do
+          now # eager-load now
+          described_class.record(42)
+        end
+
+        Gitlab::Redis.with do |redis|
+          expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]])
+        end
+      end
+    end
+
+    context 'with a time given' do
+      it 'uses the given time and records an activity in Redis' do
+        described_class.record(42, now)
+
+        Gitlab::Redis.with do |redis|
+          expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]])
+        end
+      end
+    end
+  end
+
+  describe '.delete' do
+    context 'with a single key' do
+      context 'and key exists' do
+        it 'removes the pair from Redis' do
+          described_class.record(42, now)
+
+          Gitlab::Redis.with do |redis|
+            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]])
+          end
+
+          subject.delete(42)
+
+          Gitlab::Redis.with do |redis|
+            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
+          end
+        end
+      end
+
+      context 'and key does not exist' do
+        it 'removes the pair from Redis' do
+          Gitlab::Redis.with do |redis|
+            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
+          end
+
+          subject.delete(42)
+
+          Gitlab::Redis.with do |redis|
+            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
+          end
+        end
+      end
+    end
+
+    context 'with multiple keys' do
+      context 'and all keys exist' do
+        it 'removes the pair from Redis' do
+          described_class.record(41, now)
+          described_class.record(42, now)
+
+          Gitlab::Redis.with do |redis|
+            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['41', now.to_i.to_s], ['42', now.to_i.to_s]]])
+          end
+
+          subject.delete(41, 42)
+
+          Gitlab::Redis.with do |redis|
+            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
+          end
+        end
+      end
+
+      context 'and some keys does not exist' do
+        it 'removes the existing pair from Redis' do
+          described_class.record(42, now)
+
+          Gitlab::Redis.with do |redis|
+            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', [['42', now.to_i.to_s]]])
+          end
+
+          subject.delete(41, 42)
+
+          Gitlab::Redis.with do |redis|
+            expect(redis.hscan(described_class::KEY, 0)).to eq(['0', []])
+          end
+        end
+      end
+    end
+  end
+
+  describe 'Enumerable' do
+    before do
+      described_class.record(40, now)
+      described_class.record(41, now)
+      described_class.record(42, now)
+    end
+
+    it 'allows to read the activities sequentially' do
+      expected = { '40' => now.to_i.to_s, '41' => now.to_i.to_s, '42' => now.to_i.to_s }
+
+      actual = described_class.new.each_with_object({}) do |(key, time), actual|
+        actual[key] = time
+      end
+
+      expect(actual).to eq(expected)
+    end
+
+    context 'with many records' do
+      before do
+        1_000.times { |i| described_class.record(i, now) }
+      end
+
+      it 'is possible to loop through all the records' do
+        expect(described_class.new.count).to eq(1_000)
+      end
+    end
+  end
+end

spec/migrations/migrate_user_activities_to_users_last_activity_on_spec.rb

+# encoding: utf-8
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170324160416_migrate_user_activities_to_users_last_activity_on.rb')
+
+describe MigrateUserActivitiesToUsersLastActivityOn, :redis do
+  let(:migration) { described_class.new }
+  let!(:user_active_1) { create(:user) }
+  let!(:user_active_2) { create(:user) }
+
+  def record_activity(user, time)
+    Gitlab::Redis.with do |redis|
+      redis.zadd(described_class::USER_ACTIVITY_SET_KEY, time.to_i, user.username)
+    end
+  end
+
+  around do |example|
+    Timecop.freeze { example.run }
+  end
+
+  before do
+    record_activity(user_active_1, described_class::TIME_WHEN_ACTIVITY_SET_WAS_INTRODUCED + 2.months)
+    record_activity(user_active_2, described_class::TIME_WHEN_ACTIVITY_SET_WAS_INTRODUCED + 3.months)
+    mute_stdout { migration.up }
+  end
+
+  describe '#up' do
+    it 'fills last_activity_on from the legacy Redis Sorted Set' do
+      expect(user_active_1.reload.last_activity_on).to eq((described_class::TIME_WHEN_ACTIVITY_SET_WAS_INTRODUCED + 2.months).to_date)
+      expect(user_active_2.reload.last_activity_on).to eq((described_class::TIME_WHEN_ACTIVITY_SET_WAS_INTRODUCED + 3.months).to_date)
+    end
+  end
+
+  describe '#down' do
+    it 'sets last_activity_on to NULL for all users' do
+      mute_stdout { migration.down }
+
+      expect(user_active_1.reload.last_activity_on).to be_nil
+      expect(user_active_2.reload.last_activity_on).to be_nil
+    end
+  end
+
+  def mute_stdout
+    orig_stdout = $stdout
+    $stdout = StringIO.new
+    yield
+    $stdout = orig_stdout
+  end
+end

spec/requests/api/internal_spec.rb

@@ -196,8 +196,6 @@ describe API::Internal, api: true  do
   end
 
   describe "POST /internal/allowed", :redis do
-    include UserActivitiesHelpers
-
     context "access granted" do
       before do
         project.team << [user, :developer]
@@ -215,7 +213,7 @@ describe API::Internal, api: true  do
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq(project.wiki.repository.path_to_repo)
-          expect(user_score).to be_zero
+          expect(user).not_to have_an_activity_record
         end
       end
 
@@ -226,7 +224,7 @@ describe API::Internal, api: true  do
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq(project.wiki.repository.path_to_repo)
-          expect(user_score).not_to be_zero
+          expect(user).to have_an_activity_record
         end
       end
 
@@ -237,7 +235,7 @@ describe API::Internal, api: true  do
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
-          expect(user_score).not_to be_zero
+          expect(user).to have_an_activity_record
         end
       end
 
@@ -248,7 +246,7 @@ describe API::Internal, api: true  do
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_truthy
           expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
-          expect(user_score).to be_zero
+          expect(user).not_to have_an_activity_record
         end
 
         context 'project as /namespace/project' do
@@ -284,7 +282,7 @@ describe API::Internal, api: true  do
 
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_falsey
-          expect(user_score).to be_zero
+          expect(user).not_to have_an_activity_record
         end
       end
 
@@ -294,7 +292,7 @@ describe API::Internal, api: true  do
 
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_falsey
-          expect(user_score).to be_zero
+          expect(user).not_to have_an_activity_record
         end
       end
     end
@@ -312,7 +310,7 @@ describe API::Internal, api: true  do
 
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_falsey
-          expect(user_score).to be_zero
+          expect(user).not_to have_an_activity_record
         end
       end
 
@@ -322,7 +320,7 @@ describe API::Internal, api: true  do
 
           expect(response).to have_http_status(200)
           expect(json_response["status"]).to be_falsey
-          expect(user_score).to be_zero
+          expect(user).not_to have_an_activity_record
         end
       end
     end

spec/requests/api/users_spec.rb

@@ -1171,72 +1171,44 @@ describe API::Users, api: true do
   end
 
   context "user activities", :redis do
+    let!(:old_active_user) { create(:user, last_activity_on: Time.utc(2000, 1, 1)) }
+    let!(:newly_active_user) { create(:user, last_activity_on: 2.days.ago.midday) }
+
     context 'last activity as normal user' do
       it 'has no permission' do
-        user.record_activity
-
         get api("/user/activities", user)
 
         expect(response).to have_http_status(403)
       end
     end
 
-    context 'last activity as admin' do
-      it 'returns the last activity' do
-        allow(Time).to receive(:now).and_return(Time.new(2000, 1, 1))
-
-        user.record_activity
-
+    context 'as admin' do
+      it 'returns the activities from the last 6 months' do
         get api("/user/activities", admin)
 
-        activity = json_response.last
-
         expect(response).to include_pagination_headers
-        expect(activity['username']).to eq(user.username)
-        expect(activity['last_activity_at']).to eq('2000-01-01 00:00:00')
-      end
-    end
-
-    context 'last activities paginated', :redis do
-      let(:activity) { json_response.first }
-      let(:old_date) { 2.months.ago.to_date }
-
-      before do
-        5.times do |num|
-          Timecop.freeze(old_date + num)
-
-          create(:user, username: num.to_s).record_activity
-        end
-      end
-
-      after do
-        Timecop.return
-      end
-
-      it 'returns 3 activities' do
-        get api("/user/activities?page=1&per_page=3", admin)
-
-        expect(json_response.count).to eq(3)
-      end
+        expect(json_response.size).to eq(1)
 
-      it 'contains the first activities' do
-        get api("/user/activities?page=1&per_page=3", admin)
+        activity = json_response.last
 
-        expect(json_response.map { |activity| activity['username'] }).to eq(%w[0 1 2])
+        expect(activity['username']).to eq(newly_active_user.username)
+        expect(activity['last_activity_on']).to eq(2.days.ago.to_date.to_s)
+        expect(activity['last_activity_at']).to eq(2.days.ago.to_date.to_s)
       end
 
-      it 'contains the last activities' do
-        get api("/user/activities?page=2&per_page=3", admin)
-
-        expect(json_response.map { |activity| activity['username'] }).to eq(%w[3 4])
-      end
+      context 'passing a :from parameter' do
+        it 'returns the activities from the given date' do
+          get api("/user/activities?from=2000-1-1", admin)
 
-      it 'contains activities created after user 3 was created' do
-        from = (old_date + 3).to_s("%Y-%m-%d")
+          expect(response).to include_pagination_headers
+          expect(json_response.size).to eq(2)
 
-        get api("/user/activities?page=1&per_page=5&from=#{from}", admin)
+          activity = json_response.first
 
-        expect(json_response.map { |activity| activity['username'] }).to eq(%w[3 4])
+          expect(activity['username']).to eq(old_active_user.username)
+          expect(activity['last_activity_on']).to eq(Time.utc(2000, 1, 1).to_date.to_s)
+          expect(activity['last_activity_at']).to eq(Time.utc(2000, 1, 1).to_date.to_s)
+        end
       end
     end
   end

spec/requests/git_http_spec.rb

@@ -227,8 +227,6 @@ describe 'Git HTTP requests', lib: true do
             end
 
             context "when the user isn't blocked", :redis do
-              include UserActivitiesHelpers
-
               it "responds with status 200" do
                 download(path, env) do |response|
                   expect(response.status).to eq(200)
@@ -237,7 +235,7 @@ describe 'Git HTTP requests', lib: true do
 
               it 'updates the user last activity' do
                 download(path, env) do |_response|
-                  expect(user_score).not_to be_zero
+                  expect(user).to have_an_activity_record
                 end
               end
             end

spec/services/event_create_service_spec.rb

@@ -122,7 +122,7 @@ describe EventCreateService, services: true do
     end
 
     it 'updates user last activity' do
-      expect { service.push(project, user, {}) }.to change { user_score }
+      expect { service.push(project, user, {}) }.to change { user_activity(user) }
     end
   end
 

spec/services/users/activity_service_spec.rb

@@ -14,18 +14,18 @@ describe Users::ActivityService, services: true do
       end
 
       it 'sets the last activity timestamp for the user' do
-        expect(last_hour_members).to eq([user.username])
+        expect(last_hour_user_ids).to eq([user.id])
       end
 
       it 'updates the same user' do
         service.execute
 
-        expect(last_hour_members).to eq([user.username])
+        expect(last_hour_user_ids).to eq([user.id])
       end
 
       it 'updates the timestamp of an existing user' do
         Timecop.freeze(Date.tomorrow) do
-          expect { service.execute }.to change { user_score }.to(Time.now.to_i)
+          expect { service.execute }.to change { user_activity(user) }.to(Time.now.to_i.to_s)
         end
       end
 
@@ -34,7 +34,7 @@ describe Users::ActivityService, services: true do
           other_user = create(:user)
           described_class.new(other_user, 'type').execute
 
-          expect(last_hour_members).to match_array([user.username, other_user.username])
+          expect(last_hour_user_ids).to match_array([user.id, other_user.id])
         end
       end
     end
@@ -45,8 +45,14 @@ describe Users::ActivityService, services: true do
       it 'does not update last_activity_at' do
         service.execute
 
-        expect(last_hour_members).to eq([])
+        expect(last_hour_user_ids).to eq([])
       end
     end
   end
+
+  def last_hour_user_ids
+    Gitlab::UserActivities.new.
+      select { |k, v| v >= 1.hour.ago.to_i.to_s }.
+      map { |k, _| k.to_i }
+  end
 end

spec/support/matchers/user_activity_matchers.rb

+RSpec::Matchers.define :have_an_activity_record do |expected|
+  match do |user|
+    expect(Gitlab::UserActivities.new.find { |k, _| k == user.id.to_s }).to be_present
+  end
+end

spec/support/user_activities_helpers.rb

 module UserActivitiesHelpers
-  def last_hour_members
-    Gitlab::Redis.with do |redis|
-      redis.zrangebyscore(user_activities_key, 1.hour.ago.to_i, Time.now.to_i)
-    end
-  end
-
-  def user_score
-    Gitlab::Redis.with do |redis|
-      redis.zscore(user_activities_key, user.username).to_i
-    end
-  end
-
-  def user_activities_key
-    'user/activities'
+  def user_activity(user)
+    Gitlab::UserActivities.new.
+      find { |k, _| k == user.id.to_s }&.
+      second
   end
 end

spec/workers/schedule_update_user_activity_worker_spec.rb

+require 'spec_helper'
+
+describe ScheduleUpdateUserActivityWorker, :redis do
+  let(:now) { Time.now }
+
+  before do
+    Gitlab::UserActivities.record('1', now)
+    Gitlab::UserActivities.record('2', now)
+  end
+
+  it 'schedules UpdateUserActivityWorker once' do
+    expect(UpdateUserActivityWorker).to receive(:perform_async).with({ '1' => now.to_i.to_s, '2' => now.to_i.to_s })
+
+    subject.perform
+  end
+
+  context 'when specifying a batch size' do
+    it 'schedules UpdateUserActivityWorker twice' do
+      expect(UpdateUserActivityWorker).to receive(:perform_async).with({ '1' => now.to_i.to_s })
+      expect(UpdateUserActivityWorker).to receive(:perform_async).with({ '2' => now.to_i.to_s })
+
+      subject.perform(1)
+    end
+  end
+end

spec/workers/update_user_activity_worker_spec.rb

+require 'spec_helper'
+
+describe UpdateUserActivityWorker, :redis do
+  let(:user_active_2_days_ago) { create(:user, current_sign_in_at: 10.months.ago) }
+  let(:user_active_yesterday_1) { create(:user) }
+  let(:user_active_yesterday_2) { create(:user) }
+  let(:user_active_today) { create(:user) }
+  let(:data) do
+    {
+      user_active_2_days_ago.id.to_s => 2.days.ago.at_midday.to_i.to_s,
+      user_active_yesterday_1.id.to_s => 1.day.ago.at_midday.to_i.to_s,
+      user_active_yesterday_2.id.to_s => 1.day.ago.at_midday.to_i.to_s,
+      user_active_today.id.to_s => Time.now.to_i.to_s
+    }
+  end
+
+  it 'updates users.last_activity_on' do
+    subject.perform(data)
+
+    aggregate_failures do
+      expect(user_active_2_days_ago.reload.last_activity_on).to eq(2.days.ago.to_date)
+      expect(user_active_yesterday_1.reload.last_activity_on).to eq(1.day.ago.to_date)
+      expect(user_active_yesterday_2.reload.last_activity_on).to eq(1.day.ago.to_date)
+      expect(user_active_today.reload.reload.last_activity_on).to eq(Date.today)
+    end
+  end
+
+  it 'deletes the pairs from Redis' do
+    data.each { |id, time| Gitlab::UserActivities.record(id, time) }
+
+    subject.perform(data)
+
+    expect(Gitlab::UserActivities.new.to_a).to be_empty
+  end
+end