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 823309d9d1..b46a064894 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 ) @@ -315,6 +319,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 4e933be0a6..cea214d8e4 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", "github_branch", ) @@ -317,6 +318,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..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 # 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 @@ -849,9 +852,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 3aa982e58d..d771bbad8d 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 ba543c7ae2..2c36f1ee1c 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, "github_branch": self.challenge.github_branch, } @@ -579,6 +580,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, "github_branch": self.challenge.github_branch, } @@ -683,6 +685,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, "github_branch": self.challenge.github_branch, } @@ -813,6 +816,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, "github_branch": self.challenge.github_branch, } @@ -892,6 +896,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, "github_branch": self.challenge.github_branch, } @@ -1490,6 +1495,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, "github_branch": self.challenge3.github_branch, } @@ -1575,6 +1581,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, "github_branch": self.challenge2.github_branch, } @@ -1660,6 +1667,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, "github_branch": self.challenge4.github_branch, } @@ -1745,6 +1753,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, "github_branch": self.challenge4.github_branch, }, @@ -1814,6 +1823,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, "github_branch": self.challenge3.github_branch, }, @@ -1883,6 +1893,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, "github_branch": self.challenge2.github_branch, }, @@ -2024,6 +2035,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, "github_branch": self.challenge3.github_branch, } @@ -2190,6 +2202,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, "github_branch": self.challenge3.github_branch, } @@ -2283,6 +2296,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, "github_branch": self.challenge4.github_branch, } @@ -2438,6 +2452,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, "github_branch": self.challenge2.github_branch, } @@ -2519,6 +2534,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, "github_branch": self.challenge2.github_branch, } @@ -2600,6 +2616,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, "github_branch": self.challenge2.github_branch, } @@ -2679,6 +2696,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, "github_branch": self.challenge.github_branch, }, @@ -2748,6 +2766,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, "github_branch": self.challenge2.github_branch, }, diff --git a/tests/unit/jobs/test_sender.py b/tests/unit/jobs/test_sender.py index c6598e0f28..ae60d4eaa4 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 diff --git a/tests/unit/participants/test_views.py b/tests/unit/participants/test_views.py index eaa3327fd9..ce01e99058 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, "github_branch": self.challenge1.github_branch, }, @@ -981,6 +982,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, "github_branch": self.challenge1.github_branch, } diff --git a/tests/unit/worker/test_submission_worker.py b/tests/unit/worker/test_submission_worker.py index b6fa9d2282..4559235f19 100644 --- a/tests/unit/worker/test_submission_worker.py +++ b/tests/unit/worker/test_submission_worker.py @@ -47,7 +47,7 @@ return_file_url_per_environment, run_submission, ) -from settings.common import SQS_RETENTION_PERIOD +from settings.common import SQS_RETENTION_PERIOD, SQS_VISIBILITY_TIMEOUT class BaseAPITestClass(APITestCase): @@ -294,7 +294,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")[