From ba66a7d5bdd7f429865fd8a4f4aa889c95f22ad9 Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Fri, 26 May 2023 14:10:01 -0700 Subject: [PATCH 1/4] add more details about retrying tasks --- src/docs/services/queue.mdx | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/docs/services/queue.mdx b/src/docs/services/queue.mdx index bcc0d3d4ff..9050cec2ec 100644 --- a/src/docs/services/queue.mdx +++ b/src/docs/services/queue.mdx @@ -11,14 +11,15 @@ Sentry relies on the [Celery](https://docs.celeryproject.org/) library for manag Sentry configures tasks with a special decorator that allows us more explicit control over the callable. ```python -from sentry.tasks.base import instrumented_task +from sentry.tasks.base import instrumented_task, retry @instrumented_task( name="sentry.tasks.do_work", queue="important_queue", + max_retries=3, default_retry_delay=60 * 5, - max_retries=None, ) +@retry(on=(), exclude=(), ignore=()) def do_work(kind_of_work, **kwargs): # ... ``` @@ -47,7 +48,24 @@ There are a few important points: a migration a little and gives some more operator flexibility, but message loss because of unknown arguments is still not acceptable. -- Tasks _should_ automatically retry on failure. +- Tasks _should_ retry on exception. + + Without retries, any exceptions raised while executing the task will stop + the task from moving forward. Generally, there are three categories of + exceptions that could be raised: + - Retryable exceptions: transient errors (e.g. connection timeouts, + DB unavailable, query read timeouts) + - Ignorable exceptions: this is task specific but an example of an + ignorable exception would be if your task is attempting to delete a row + but the row doesn't exist and raises `DoesNotExist` exception. + - Non-ignorable programming errors that should be re-raised and fixed + (AttributeError, TypeError, ValueError, etc) + + Use the `@retry` decorator to specify which exceptions should be retried, + re-raised, and ignored. It might not be possible to know ahead of time + all the possible exceptions that could be raised in your task, so starting + with just the `@retry(on=())` might make sense and + updating the types of exceptions to handle after encountering them. - Tasks arguments _should_ be primitive types and small. @@ -70,6 +88,9 @@ There are a few important points: containing a task must be added to the `CELERY_IMPORTS` setting in `src/sentry/conf/server.py`. +- Tasks specifying a named `queue` must be added to `CELERY_QUEUES`. + + ## Running a Worker Workers can be run by using the [Sentry CLI](https://docs.sentry.io/product/cli/). From 8e6a1f82cecbf4b2bcd44dfc0a7c791bf6be83ec Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Fri, 26 May 2023 14:12:34 -0700 Subject: [PATCH 2/4] consistency --- src/docs/services/queue.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/docs/services/queue.mdx b/src/docs/services/queue.mdx index 9050cec2ec..f58da2432e 100644 --- a/src/docs/services/queue.mdx +++ b/src/docs/services/queue.mdx @@ -19,7 +19,7 @@ from sentry.tasks.base import instrumented_task, retry max_retries=3, default_retry_delay=60 * 5, ) -@retry(on=(), exclude=(), ignore=()) +@retry(on=(), exclude=(), ignore=()) def do_work(kind_of_work, **kwargs): # ... ``` From 7e96483e15cdec354e0b5d40e180d1cd7cf70c26 Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Fri, 26 May 2023 14:15:50 -0700 Subject: [PATCH 3/4] extra tilda --- src/docs/services/queue.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/docs/services/queue.mdx b/src/docs/services/queue.mdx index f58da2432e..79af4aa1c6 100644 --- a/src/docs/services/queue.mdx +++ b/src/docs/services/queue.mdx @@ -34,7 +34,7 @@ There are a few important points: names which makes the name tied to the location of the code and more brittle for future code maintenance. -- Tasks _must_ accept `\*\*kwargs`` to handle rolling compatibility. +- Tasks _must_ accept `\*\*kwargs` to handle rolling compatibility. This ensures tasks will accept any message which happens to be in the queue rather than fail for unknown arguments. It helps with From 2216cbba10c7ef4706a685eef6bb0f6c127f823b Mon Sep 17 00:00:00 2001 From: Gilbert Szeto Date: Fri, 26 May 2023 14:21:30 -0700 Subject: [PATCH 4/4] markdown nested list --- src/docs/services/queue.mdx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/docs/services/queue.mdx b/src/docs/services/queue.mdx index 79af4aa1c6..f0ed92b98f 100644 --- a/src/docs/services/queue.mdx +++ b/src/docs/services/queue.mdx @@ -34,7 +34,7 @@ There are a few important points: names which makes the name tied to the location of the code and more brittle for future code maintenance. -- Tasks _must_ accept `\*\*kwargs` to handle rolling compatibility. +- Tasks _must_ accept `kwargs` to handle rolling compatibility. This ensures tasks will accept any message which happens to be in the queue rather than fail for unknown arguments. It helps with @@ -53,12 +53,13 @@ There are a few important points: Without retries, any exceptions raised while executing the task will stop the task from moving forward. Generally, there are three categories of exceptions that could be raised: - - Retryable exceptions: transient errors (e.g. connection timeouts, + + 1. Retryable exceptions: transient errors (e.g. connection timeouts, DB unavailable, query read timeouts) - - Ignorable exceptions: this is task specific but an example of an + 2. Ignorable exceptions: this is task specific but an example of an ignorable exception would be if your task is attempting to delete a row but the row doesn't exist and raises `DoesNotExist` exception. - - Non-ignorable programming errors that should be re-raised and fixed + 3. Non-ignorable programming errors that should be re-raised and fixed (AttributeError, TypeError, ValueError, etc) Use the `@retry` decorator to specify which exceptions should be retried,