From 504e57a3148690b679dae2ee7d4b9712ae8211ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ozren=20Dabi=C4=87?= Date: Sat, 7 Jun 2025 18:30:53 +0200 Subject: [PATCH 1/3] Introduce SQS `VisibilityTimeout` --- apps/base/utils.py | 12 +++++- apps/challenges/aws_utils.py | 39 +++++++++++++++++++ .../0113_challenge_sqs_visibility_timeout.py | 20 ++++++++++ apps/challenges/models.py | 21 ++++++++++ apps/challenges/serializers.py | 2 + apps/jobs/sender.py | 12 +++++- scripts/workers/submission_worker.py | 12 +++++- settings/common.py | 2 + tests/unit/challenges/test_views.py | 19 +++++++++ tests/unit/participants/test_views.py | 2 + tests/unit/worker/test_submission_worker.py | 7 +++- 11 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 apps/challenges/migrations/0113_challenge_sqs_visibility_timeout.py diff --git a/apps/base/utils.py b/apps/base/utils.py index 4d487f5c8c..e051af0962 100644 --- a/apps/base/utils.py +++ b/apps/base/utils.py @@ -16,7 +16,7 @@ from rest_framework.pagination import PageNumberPagination from sendgrid.helpers.mail import Email, Mail, Personalization -from settings.common import SQS_RETENTION_PERIOD +from settings.common import SQS_RETENTION_PERIOD, SQS_VISIBILITY_TIMEOUT logger = logging.getLogger(__name__) @@ -206,9 +206,17 @@ def get_or_create_sqs_queue(queue_name, challenge=None): if challenge is None else str(challenge.sqs_retention_period) ) + sqs_visibility_timeout = ( + SQS_VISIBILITY_TIMEOUT + if challenge is None + else str(challenge.sqs_visibility_timeout) + ) queue = sqs.create_queue( QueueName=queue_name, - Attributes={"MessageRetentionPeriod": sqs_retention_period}, + Attributes={ + "MessageRetentionPeriod": sqs_retention_period, + "VisibilityTimeout": sqs_visibility_timeout, + }, ) return queue diff --git a/apps/challenges/aws_utils.py b/apps/challenges/aws_utils.py index 622756d9c1..58c8f5ea31 100644 --- a/apps/challenges/aws_utils.py +++ b/apps/challenges/aws_utils.py @@ -839,6 +839,32 @@ def update_sqs_retention_period(challenge): } +def update_sqs_visibility_timeout(challenge): + """ + Update the SQS visibility timeout for a challenge. + + Args: + challenge (Challenge): The challenge for which the SQS visibility timeout is to be updated. + + Returns: + dict: A dictionary containing the status and message of the operation. + """ + sqs_visibility_timeout = str(challenge.sqs_visibility_timeout) + try: + sqs = get_boto3_client("sqs", aws_keys) + queue_url = sqs.get_queue_url(QueueName=challenge.queue)["QueueUrl"] + response = sqs.set_queue_attributes( + QueueUrl=queue_url, + Attributes={"VisibilityTimeout": sqs_visibility_timeout}, + ) + return {"message": response} + except Exception as e: + logger.exception(e) + return { + "error": str(e), + } + + def start_workers(queryset): """ The function called by the admin action method to start all the selected workers. @@ -1871,3 +1897,16 @@ def update_sqs_retention_period_task(challenge): for obj in serializers.deserialize("json", challenge): challenge_obj = obj.object return update_sqs_retention_period(challenge_obj) + + +@app.task +def update_sqs_visibility_timeout_task(challenge): + """ + Updates sqs visibility timeout for a challenge when the attribute is changed. + + Args: + challenge: {} -- instance of the model calling the post hook + """ + for obj in serializers.deserialize("json", challenge): + challenge_obj = obj.object + return update_sqs_visibility_timeout(challenge_obj) diff --git a/apps/challenges/migrations/0113_challenge_sqs_visibility_timeout.py b/apps/challenges/migrations/0113_challenge_sqs_visibility_timeout.py new file mode 100644 index 0000000000..6f27b1c073 --- /dev/null +++ b/apps/challenges/migrations/0113_challenge_sqs_visibility_timeout.py @@ -0,0 +1,20 @@ +# Generated by Django 2.2.20 on 2025-06-07 18:52 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("challenges", "0112_challenge_sqs_retention_period"), + ] + + operations = [ + migrations.AddField( + model_name="challenge", + name="sqs_visibility_timeout", + field=models.PositiveIntegerField( + default=300, verbose_name="SQS Visibility Timeout" + ), + ), + ] diff --git a/apps/challenges/models.py b/apps/challenges/models.py index 919c95209f..dcddb72326 100644 --- a/apps/challenges/models.py +++ b/apps/challenges/models.py @@ -32,6 +32,7 @@ def __init__(self, *args, **kwargs): self._original_evaluation_script = self.evaluation_script self._original_approved_by_admin = self.approved_by_admin self._original_sqs_retention_period = self.sqs_retention_period + self._original_sqs_visibility_timeout = self.sqs_visibility_timeout title = models.CharField(max_length=100, db_index=True) short_description = models.TextField(null=True, blank=True) @@ -134,6 +135,9 @@ def __init__(self, *args, **kwargs): sqs_retention_period = models.PositiveIntegerField( default=345600, verbose_name="SQS Retention Period" ) + sqs_visibility_timeout = models.PositiveIntegerField( + default=300, verbose_name="SQS Visibility Timeout" + ) is_docker_based = models.BooleanField( default=False, verbose_name="Is Docker Based", db_index=True ) @@ -311,6 +315,23 @@ def update_sqs_retention_period_for_challenge( challenge.save() +@receiver(signals.post_save, sender="challenges.Challenge") +def update_sqs_visibility_timeout_for_challenge( + sender, instance, created, **kwargs +): + field_name = "sqs_visibility_timeout" + import challenges.aws_utils as aws + + if not created and is_model_field_changed(instance, field_name): + serialized_obj = serializers.serialize("json", [instance]) + aws.update_sqs_visibility_timeout_task.delay(serialized_obj) + # Update challenge + curr = getattr(instance, "{}".format(field_name)) + challenge = instance + challenge._original_sqs_visibility_timeout = curr + challenge.save() + + class DatasetSplit(TimeStampedModel): name = models.CharField(max_length=100) codename = models.CharField(max_length=100) diff --git a/apps/challenges/serializers.py b/apps/challenges/serializers.py index 8be657de57..f4471bbb29 100644 --- a/apps/challenges/serializers.py +++ b/apps/challenges/serializers.py @@ -94,6 +94,7 @@ class Meta: "worker_image_url", "worker_instance_type", "sqs_retention_period", + "sqs_visibility_timeout", "github_repository", ) @@ -312,6 +313,7 @@ class Meta: "evaluation_module_error", "worker_image_url", "sqs_retention_period", + "sqs_visibility_timeout", ) diff --git a/apps/jobs/sender.py b/apps/jobs/sender.py index e0b5efd2c9..f51fa79f71 100644 --- a/apps/jobs/sender.py +++ b/apps/jobs/sender.py @@ -14,7 +14,7 @@ NUM_SUBMISSIONS_IN_QUEUE, increment_statsd_counter, ) -from settings.common import SQS_RETENTION_PERIOD +from settings.common import SQS_RETENTION_PERIOD, SQS_VISIBILITY_TIMEOUT from .utils import get_submission_model @@ -70,9 +70,17 @@ def get_or_create_sqs_queue(queue_name, challenge=None): if challenge is None else str(challenge.sqs_retention_period) ) + sqs_visibility_timeout = ( + SQS_VISIBILITY_TIMEOUT + if challenge is None + else str(challenge.sqs_visibility_timeout) + ) queue = sqs.create_queue( QueueName=queue_name, - Attributes={"MessageRetentionPeriod": sqs_retention_period}, + Attributes={ + "MessageRetentionPeriod": sqs_retention_period, + "VisibilityTimeout": sqs_visibility_timeout, + }, ) else: logger.exception("Cannot get or create Queue") diff --git a/scripts/workers/submission_worker.py b/scripts/workers/submission_worker.py index 8de368adab..6ef364ac8b 100644 --- a/scripts/workers/submission_worker.py +++ b/scripts/workers/submission_worker.py @@ -44,7 +44,7 @@ from jobs.models import Submission # noqa:E402 from jobs.serializers import SubmissionSerializer # noqa:E402 -from settings.common import SQS_RETENTION_PERIOD # noqa:E402 +from settings.common import SQS_RETENTION_PERIOD, SQS_VISIBILITY_TIMEOUT # noqa:E402 from .statsd_utils import increment_and_push_metrics_to_statsd # noqa:E402 @@ -849,9 +849,17 @@ def get_or_create_sqs_queue(queue_name, challenge=None): if challenge is None else str(challenge.sqs_retention_period) ) + sqs_visibility_timeout = ( + SQS_VISIBILITY_TIMEOUT + if challenge is None + else str(challenge.sqs_visibility_timeout) + ) queue = sqs.create_queue( QueueName=queue_name, - Attributes={"MessageRetentionPeriod": sqs_retention_period}, + Attributes={ + "MessageRetentionPeriod": sqs_retention_period, + "VisibilityTimeout": sqs_visibility_timeout, + }, ) return queue diff --git a/settings/common.py b/settings/common.py index 7ee3c546bc..93f736adef 100755 --- a/settings/common.py +++ b/settings/common.py @@ -415,3 +415,5 @@ # SQS Queue Message Retention Period SQS_RETENTION_PERIOD = "345600" +# SQS Queue Message Visibility Timeout +SQS_VISIBILITY_TIMEOUT = "300" diff --git a/tests/unit/challenges/test_views.py b/tests/unit/challenges/test_views.py index 91c7a04ee3..40f9a770d6 100644 --- a/tests/unit/challenges/test_views.py +++ b/tests/unit/challenges/test_views.py @@ -204,6 +204,7 @@ def test_get_challenge(self): "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, "sqs_retention_period": self.challenge.sqs_retention_period, + "sqs_visibility_timeout": self.challenge.sqs_visibility_timeout, "github_repository": self.challenge.github_repository, } ] @@ -576,6 +577,7 @@ def test_get_particular_challenge(self): "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, "sqs_retention_period": self.challenge.sqs_retention_period, + "sqs_visibility_timeout": self.challenge.sqs_visibility_timeout, "github_repository": self.challenge.github_repository, } response = self.client.get(self.url, {}) @@ -679,6 +681,7 @@ def test_update_challenge_when_user_is_its_creator(self): "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, "sqs_retention_period": self.challenge.sqs_retention_period, + "sqs_visibility_timeout": self.challenge.sqs_visibility_timeout, "github_repository": self.challenge.github_repository, } response = self.client.put( @@ -808,6 +811,7 @@ def test_particular_challenge_partial_update(self): "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, "sqs_retention_period": self.challenge.sqs_retention_period, + "sqs_visibility_timeout": self.challenge.sqs_visibility_timeout, "github_repository": self.challenge.github_repository, } response = self.client.patch(self.url, self.partial_update_data) @@ -886,6 +890,7 @@ def test_particular_challenge_update(self): "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, "sqs_retention_period": self.challenge.sqs_retention_period, + "sqs_visibility_timeout": self.challenge.sqs_visibility_timeout, "github_repository": self.challenge.github_repository, } response = self.client.put(self.url, self.data) @@ -1483,6 +1488,7 @@ def test_get_past_challenges(self): "worker_image_url": self.challenge3.worker_image_url, "worker_instance_type": self.challenge3.worker_instance_type, "sqs_retention_period": self.challenge3.sqs_retention_period, + "sqs_visibility_timeout": self.challenge3.sqs_visibility_timeout, "github_repository": self.challenge3.github_repository, } ] @@ -1567,6 +1573,7 @@ def test_get_present_challenges(self): "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, "sqs_retention_period": self.challenge2.sqs_retention_period, + "sqs_visibility_timeout": self.challenge2.sqs_visibility_timeout, "github_repository": self.challenge2.github_repository, } ] @@ -1651,6 +1658,7 @@ def test_get_future_challenges(self): "worker_image_url": self.challenge4.worker_image_url, "worker_instance_type": self.challenge4.worker_instance_type, "sqs_retention_period": self.challenge4.sqs_retention_period, + "sqs_visibility_timeout": self.challenge4.sqs_visibility_timeout, "github_repository": self.challenge4.github_repository, } ] @@ -1735,6 +1743,7 @@ def test_get_all_challenges(self): "worker_image_url": self.challenge4.worker_image_url, "worker_instance_type": self.challenge4.worker_instance_type, "sqs_retention_period": self.challenge4.sqs_retention_period, + "sqs_visibility_timeout": self.challenge4.sqs_visibility_timeout, "github_repository": self.challenge4.github_repository, }, { @@ -1803,6 +1812,7 @@ def test_get_all_challenges(self): "worker_image_url": self.challenge3.worker_image_url, "worker_instance_type": self.challenge3.worker_instance_type, "sqs_retention_period": self.challenge3.sqs_retention_period, + "sqs_visibility_timeout": self.challenge3.sqs_visibility_timeout, "github_repository": self.challenge3.github_repository, }, { @@ -1871,6 +1881,7 @@ def test_get_all_challenges(self): "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, "sqs_retention_period": self.challenge2.sqs_retention_period, + "sqs_visibility_timeout": self.challenge2.sqs_visibility_timeout, "github_repository": self.challenge2.github_repository, }, ] @@ -2011,6 +2022,7 @@ def test_get_featured_challenges(self): "worker_image_url": self.challenge3.worker_image_url, "worker_instance_type": self.challenge3.worker_instance_type, "sqs_retention_period": self.challenge3.sqs_retention_period, + "sqs_visibility_timeout": self.challenge3.sqs_visibility_timeout, "github_repository": self.challenge3.github_repository, } ] @@ -2176,6 +2188,7 @@ def test_get_challenge_by_pk_when_user_is_challenge_host(self): "worker_image_url": self.challenge3.worker_image_url, "worker_instance_type": self.challenge3.worker_instance_type, "sqs_retention_period": self.challenge3.sqs_retention_period, + "sqs_visibility_timeout": self.challenge3.sqs_visibility_timeout, "github_repository": self.challenge3.github_repository, } @@ -2268,6 +2281,7 @@ def test_get_challenge_by_pk_when_user_is_participant(self): "worker_image_url": self.challenge4.worker_image_url, "worker_instance_type": self.challenge4.worker_instance_type, "sqs_retention_period": self.challenge4.sqs_retention_period, + "sqs_visibility_timeout": self.challenge4.sqs_visibility_timeout, "github_repository": self.challenge4.github_repository, } @@ -2422,6 +2436,7 @@ def test_get_challenge_when_host_team_is_given(self): "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, "sqs_retention_period": self.challenge2.sqs_retention_period, + "sqs_visibility_timeout": self.challenge2.sqs_visibility_timeout, "github_repository": self.challenge2.github_repository, } ] @@ -2502,6 +2517,7 @@ def test_get_challenge_when_participant_team_is_given(self): "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, "sqs_retention_period": self.challenge2.sqs_retention_period, + "sqs_visibility_timeout": self.challenge2.sqs_visibility_timeout, "github_repository": self.challenge2.github_repository, } ] @@ -2582,6 +2598,7 @@ def test_get_challenge_when_mode_is_participant(self): "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, "sqs_retention_period": self.challenge2.sqs_retention_period, + "sqs_visibility_timeout": self.challenge2.sqs_visibility_timeout, "github_repository": self.challenge2.github_repository, } ] @@ -2660,6 +2677,7 @@ def test_get_challenge_when_mode_is_host(self): "worker_image_url": self.challenge.worker_image_url, "worker_instance_type": self.challenge.worker_instance_type, "sqs_retention_period": self.challenge.sqs_retention_period, + "sqs_visibility_timeout": self.challenge.sqs_visibility_timeout, "github_repository": self.challenge.github_repository, }, { @@ -2728,6 +2746,7 @@ def test_get_challenge_when_mode_is_host(self): "worker_image_url": self.challenge2.worker_image_url, "worker_instance_type": self.challenge2.worker_instance_type, "sqs_retention_period": self.challenge2.sqs_retention_period, + "sqs_visibility_timeout": self.challenge.sqs_visibility_timeout, "github_repository": self.challenge2.github_repository, }, ] diff --git a/tests/unit/participants/test_views.py b/tests/unit/participants/test_views.py index 89539ede1f..13df7e6e1c 100644 --- a/tests/unit/participants/test_views.py +++ b/tests/unit/participants/test_views.py @@ -883,6 +883,7 @@ def test_get_teams_and_corresponding_challenges_for_a_participant(self): "worker_image_url": self.challenge1.worker_image_url, "worker_instance_type": self.challenge1.worker_instance_type, "sqs_retention_period": self.challenge1.sqs_retention_period, + "sqs_visibility_timeout": self.challenge1.sqs_visibility_timeout, "github_repository": self.challenge1.github_repository, }, "participant_team": { @@ -980,6 +981,7 @@ def test_get_participant_team_challenge_list(self): "worker_image_url": self.challenge1.worker_image_url, "worker_instance_type": self.challenge1.worker_instance_type, "sqs_retention_period": self.challenge1.sqs_retention_period, + "sqs_visibility_timeout": self.challenge1.sqs_visibility_timeout, "github_repository": self.challenge1.github_repository, } ] diff --git a/tests/unit/worker/test_submission_worker.py b/tests/unit/worker/test_submission_worker.py index 05258b8211..48417992cc 100644 --- a/tests/unit/worker/test_submission_worker.py +++ b/tests/unit/worker/test_submission_worker.py @@ -39,7 +39,7 @@ load_challenge_and_return_max_submissions, return_file_url_per_environment, ) -from settings.common import SQS_RETENTION_PERIOD +from settings.common import SQS_RETENTION_PERIOD, SQS_VISIBILITY_TIMEOUT class BaseAPITestClass(APITestCase): @@ -287,7 +287,10 @@ def test_load_challenge_and_return_max_submissions_when_challenge_does_not_exist def test_get_or_create_sqs_queue_for_existing_queue(self): self.sqs_client.create_queue( QueueName="test_queue", - Attributes={"MessageRetentionPeriod": SQS_RETENTION_PERIOD}, + Attributes={ + "MessageRetentionPeriod": SQS_RETENTION_PERIOD, + "VisibilityTimeout": SQS_VISIBILITY_TIMEOUT, + }, ) get_or_create_sqs_queue("test_queue") queue_url = self.sqs_client.get_queue_url(QueueName="test_queue")[ From 9bf928259d3497b0a170688475083c1512bf4552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ozren=20Dabi=C4=87?= Date: Sat, 7 Jun 2025 19:55:49 +0200 Subject: [PATCH 2/3] Fix missed test assertion --- tests/unit/jobs/test_sender.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/jobs/test_sender.py b/tests/unit/jobs/test_sender.py index 47563c08cf..d8fcb0da1e 100644 --- a/tests/unit/jobs/test_sender.py +++ b/tests/unit/jobs/test_sender.py @@ -192,6 +192,7 @@ def test_get_or_create_sqs_queue_non_existent_queue( mock_challenge = MagicMock() mock_challenge.use_host_sqs = False mock_challenge.sqs_retention_period = "1209600" + mock_challenge.sqs_visibility_timeout = "600" mock_sqs = MagicMock() mock_boto3_resource.return_value = mock_sqs @@ -210,7 +211,8 @@ def test_get_or_create_sqs_queue_non_existent_queue( mock_sqs.create_queue.assert_called_once_with( QueueName=queue_name, Attributes={ - "MessageRetentionPeriod": mock_challenge.sqs_retention_period + "MessageRetentionPeriod": mock_challenge.sqs_retention_period, + "VisibilityTimeout": mock_challenge.sqs_visibility_timeout, }, ) assert queue == mock_created_queue From fcff556de209eec37d570284e2ece1fcac996606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ozren=20Dabi=C4=87?= Date: Sat, 7 Jun 2025 21:50:13 +0200 Subject: [PATCH 3/3] Fix linter issue in `submission_worker.py` --- scripts/workers/submission_worker.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/workers/submission_worker.py b/scripts/workers/submission_worker.py index 6ef364ac8b..5079e9da7d 100644 --- a/scripts/workers/submission_worker.py +++ b/scripts/workers/submission_worker.py @@ -44,7 +44,10 @@ from jobs.models import Submission # noqa:E402 from jobs.serializers import SubmissionSerializer # noqa:E402 -from settings.common import SQS_RETENTION_PERIOD, SQS_VISIBILITY_TIMEOUT # noqa:E402 +from settings.common import ( # noqa:E402 + SQS_RETENTION_PERIOD, + SQS_VISIBILITY_TIMEOUT, +) from .statsd_utils import increment_and_push_metrics_to_statsd # noqa:E402