From aaca3948132c2ac078097e00093fa6598d8d2e0b Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Wed, 31 Jul 2024 22:17:53 +0200 Subject: [PATCH] presence: Remove the queue worker. --- docs/production/troubleshooting.md | 1 - docs/subsystems/queuing.md | 2 +- .../kandra/files/nagios4/conf.d/services.cfg | 6 ----- puppet/zulip/manifests/app_frontend_base.pp | 1 - scripts/lib/check_rabbitmq_queue.py | 1 - zerver/actions/presence.py | 18 +++++-------- zerver/tests/test_presence.py | 17 ++++++------ zerver/worker/user_presence.py | 26 ------------------- 8 files changed, 16 insertions(+), 56 deletions(-) delete mode 100644 zerver/worker/user_presence.py diff --git a/docs/production/troubleshooting.md b/docs/production/troubleshooting.md index d2cffba784..cdab29897a 100644 --- a/docs/production/troubleshooting.md +++ b/docs/production/troubleshooting.md @@ -91,7 +91,6 @@ zulip-workers:zulip_events_missedmessage_mobile_notifications RUNNING pid 11 zulip-workers:zulip_events_outgoing_webhooks RUNNING pid 11358, uptime 19:40:17 zulip-workers:zulip_events_user_activity RUNNING pid 11365, uptime 19:40:14 zulip-workers:zulip_events_user_activity_interval RUNNING pid 11376, uptime 19:40:11 -zulip-workers:zulip_events_user_presence RUNNING pid 11384, uptime 19:40:08 ``` If you see any services showing a status other than `RUNNING`, or you diff --git a/docs/subsystems/queuing.md b/docs/subsystems/queuing.md index f76f1471f7..9e436f2a0c 100644 --- a/docs/subsystems/queuing.md +++ b/docs/subsystems/queuing.md @@ -83,7 +83,7 @@ apt and then run: ```bash amqp-delete-queue --username=zulip --password='...' --server=localhost \ - --queue=user_presence + --queue=user_activity ``` with the RabbitMQ password from `/etc/zulip/zulip-secrets.conf`. diff --git a/puppet/kandra/files/nagios4/conf.d/services.cfg b/puppet/kandra/files/nagios4/conf.d/services.cfg index 6e55cef933..bb2f81e34e 100644 --- a/puppet/kandra/files/nagios4/conf.d/services.cfg +++ b/puppet/kandra/files/nagios4/conf.d/services.cfg @@ -394,12 +394,6 @@ define service { check_command check_rabbitmq_consumers!user_activity_interval } -define service { - use rabbitmq-consumer-service - service_description Check RabbitMQ user_presence consumers - check_command check_rabbitmq_consumers!user_presence -} - define service { use generic-service service_description Check worker memory usage diff --git a/puppet/zulip/manifests/app_frontend_base.pp b/puppet/zulip/manifests/app_frontend_base.pp index 2396f172a8..fe0f957d03 100644 --- a/puppet/zulip/manifests/app_frontend_base.pp +++ b/puppet/zulip/manifests/app_frontend_base.pp @@ -144,7 +144,6 @@ class zulip::app_frontend_base { 'thumbnail', 'user_activity', 'user_activity_interval', - 'user_presence', ] if $zulip::common::total_memory_mb > 24000 { diff --git a/scripts/lib/check_rabbitmq_queue.py b/scripts/lib/check_rabbitmq_queue.py index 4307409d0c..ef6c6c2179 100644 --- a/scripts/lib/check_rabbitmq_queue.py +++ b/scripts/lib/check_rabbitmq_queue.py @@ -25,7 +25,6 @@ normal_queues = [ "thumbnail", "user_activity", "user_activity_interval", - "user_presence", ] mobile_notification_shards = int( diff --git a/zerver/actions/presence.py b/zerver/actions/presence.py index 3fa56ff248..d5cc9378e7 100644 --- a/zerver/actions/presence.py +++ b/zerver/actions/presence.py @@ -11,8 +11,6 @@ from zerver.lib.presence import ( format_legacy_presence_dict, user_presence_datetime_with_date_joined_default, ) -from zerver.lib.queue import queue_json_publish -from zerver.lib.timestamp import datetime_to_timestamp from zerver.lib.users import get_user_ids_who_can_access_user from zerver.models import Client, UserPresence, UserProfile from zerver.models.clients import get_client @@ -286,14 +284,12 @@ def update_user_presence( status: int, new_user_input: bool, ) -> None: - event = { - "user_profile_id": user_profile.id, - "status": status, - "time": datetime_to_timestamp(log_time), - "client": client.name, - } - - queue_json_publish("user_presence", event) - + logger.debug( + "Processing presence update for user %s, client %s, status %s", + user_profile.id, + client, + status, + ) + do_update_user_presence(user_profile, client, log_time, status) if new_user_input: update_user_activity_interval(user_profile, log_time) diff --git a/zerver/tests/test_presence.py b/zerver/tests/test_presence.py index d0e64bec5f..3c7b89ad55 100644 --- a/zerver/tests/test_presence.py +++ b/zerver/tests/test_presence.py @@ -263,9 +263,6 @@ class UserPresenceTests(ZulipTestCase): # In tests, the presence update is processed immediately rather than in the background # in the queue worker, so we see it reflected immediately. - # In production, our presence update may be processed with some delay, so the last_update_id - # might not include it yet. In such a case, we'd see the original value of -1 returned, - # due to there being no new data to return. last_update_id = UserPresence.objects.latest("last_update_id").last_update_id self.assertEqual(json["presence_last_update_id"], last_update_id) @@ -323,12 +320,14 @@ class UserPresenceTests(ZulipTestCase): UserPresence.objects.all().delete() params = dict(status="idle", last_update_id=-1) - # Make do_update_user_presence a noop. This simulates a production-like environment - # where the update is processed in a queue worker, so hamlet may not see his update - # reflected back to him in the response. Therefore it is as if there is no presence - # data. + # Make do_update_user_presence a noop. This simulates a scenario as if there + # is no presence data. + # This is not a realistic situation, because the presence update that the user + # is making will by itself bump the last_update_id which will be reflected + # here in the response - but it's still good to test the code is robust + # and works fine in such an edge case. # In such a situation, he should get his last_update_id=-1 back. - with mock.patch("zerver.worker.user_presence.do_update_user_presence"): + with mock.patch("zerver.actions.presence.do_update_user_presence"): result = self.client_post("/json/users/me/presence", params) json = self.assert_json_success(result) @@ -340,7 +339,7 @@ class UserPresenceTests(ZulipTestCase): # like an old slim_presence client would due to an implementation # prior to the introduction of last_update_id. params = dict(status="idle") - with mock.patch("zerver.worker.user_presence.do_update_user_presence"): + with mock.patch("zerver.actions.presence.do_update_user_presence"): result = self.client_post("/json/users/me/presence", params) json = self.assert_json_success(result) self.assertEqual(set(json["presences"].keys()), set()) diff --git a/zerver/worker/user_presence.py b/zerver/worker/user_presence.py deleted file mode 100644 index 5239643dd7..0000000000 --- a/zerver/worker/user_presence.py +++ /dev/null @@ -1,26 +0,0 @@ -# Documented in https://zulip.readthedocs.io/en/latest/subsystems/queuing.html -import logging -from collections.abc import Mapping -from typing import Any - -from typing_extensions import override - -from zerver.actions.presence import do_update_user_presence -from zerver.lib.timestamp import timestamp_to_datetime -from zerver.models.clients import get_client -from zerver.models.users import get_user_profile_by_id -from zerver.worker.base import QueueProcessingWorker, assign_queue - -logger = logging.getLogger(__name__) - - -@assign_queue("user_presence") -class UserPresenceWorker(QueueProcessingWorker): - @override - def consume(self, event: Mapping[str, Any]) -> None: - logging.debug("Received presence event: %s", event) - user_profile = get_user_profile_by_id(event["user_profile_id"]) - client = get_client(event["client"]) - log_time = timestamp_to_datetime(event["time"]) - status = event["status"] - do_update_user_presence(user_profile, client, log_time, status)