Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The delete_expired method of ResultManager sometimes selects read-only database #357

Open
jacklinke opened this issue Jan 16, 2023 · 0 comments · May be fixed by #358
Open

The delete_expired method of ResultManager sometimes selects read-only database #357

jacklinke opened this issue Jan 16, 2023 · 0 comments · May be fixed by #358

Comments

@jacklinke
Copy link

In a django project with a database router and read-only databases, the delete_expired method may try to perform raw_delete() using a read-only database, resulting in an Internal Error: "ReadOnlySqlTransaction - cannot execute DELETE in a read-only transaction".

Relevant code:

def delete_expired(self, expires):
    """Delete all expired results."""
    with transaction.atomic(using=self.db):
        raw_delete(queryset=self.get_all_expired(expires))

My Database Router:

import random

from django.conf import settings


class CustomRouter:
    def db_for_read(self, model, **hints):
        """
        Return either default or one of the replicas
        """

        db_read = [
            "default",
        ]

        if settings.REPLICA1_URL is not None:
            db_read.append("replica1")
        if settings.REPLICA2_URL is not None:
            db_read.append("replica2")

        return random.choice(db_read)

    def db_for_write(self, model, **hints):
        # Always return the default database
        return "default"

    def allow_relation(self, obj1, obj2, **hints):
        return True

    def allow_migrate(self, db, app_label, model_name=None, **hints):
        return True

Occasionally, django-celery-results will attempt to conduct delete_expired using one of my replica databases:

connection: <DatabaseWrapper vendor='postgresql' alias='replica1'>

Recommend changing the using argument of the atomic transaction in delete_expired to explicitly use the writable database specified in the router.

@jacklinke jacklinke linked a pull request Jan 16, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant