From def3adf9670c52823b5f3ac7ed55a420f422f145 Mon Sep 17 00:00:00 2001 From: Dawid Wolski Date: Sun, 25 Apr 2021 00:48:02 +0200 Subject: [PATCH 1/3] Batch tokens deletion in cleartokens command --- docs/management_commands.rst | 3 ++ docs/settings.rst | 12 +++++++ oauth2_provider/models.py | 61 ++++++++++++++++++++++++------------ oauth2_provider/settings.py | 2 ++ tests/settings.py | 3 ++ 5 files changed, 61 insertions(+), 20 deletions(-) diff --git a/docs/management_commands.rst b/docs/management_commands.rst index 3930062b6..147a0bbe4 100644 --- a/docs/management_commands.rst +++ b/docs/management_commands.rst @@ -16,5 +16,8 @@ If ``cleartokens`` runs daily the maximum delay before a refresh token is removed is ``REFRESH_TOKEN_EXPIRE_SECONDS`` + 1 day. This is normally not a problem since refresh tokens are long lived. +To prevent the CPU and RAM high peaks during deletion process use ``CLEAR_EXPIRED_TOKENS_BATCH_SIZE`` and +``CLEAR_EXPIRED_TOKENS_BATCH_INTERVAL`` settings to adjust the process speed. + Note: Refresh tokens need to expire before AccessTokens can be removed from the database. Using ``cleartokens`` without ``REFRESH_TOKEN_EXPIRE_SECONDS`` has limited effect. diff --git a/docs/settings.rst b/docs/settings.rst index 07561d3d2..49460bc0e 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -337,6 +337,18 @@ Default: ``["client_secret_post", "client_secret_basic"]`` The authentication methods that are advertised to be supported by this server. +CLEAR_EXPIRED_TOKENS_BATCH_SIZE +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Default: ``10000`` + +The size of delete batches used by ``cleartokens`` management command. + +CLEAR_EXPIRED_TOKENS_BATCH_INTERVAL +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Default: ``0.1`` + +Time of sleep in seconds used by ``cleartokens`` management command between batch deletions. + Settings imported from Django project -------------------------- diff --git a/oauth2_provider/models.py b/oauth2_provider/models.py index d7b767a78..2c9747ce8 100644 --- a/oauth2_provider/models.py +++ b/oauth2_provider/models.py @@ -1,4 +1,5 @@ import logging +import time import uuid from datetime import timedelta from urllib.parse import parse_qsl, urlparse @@ -621,12 +622,31 @@ def get_refresh_token_admin_class(): def clear_expired(): + def batch_delete(queryset, query): + CLEAR_EXPIRED_TOKENS_BATCH_SIZE = oauth2_settings.CLEAR_EXPIRED_TOKENS_BATCH_SIZE + CLEAR_EXPIRED_TOKENS_BATCH_INTERVAL = oauth2_settings.CLEAR_EXPIRED_TOKENS_BATCH_INTERVAL + current_no = start_no = queryset.count() + + while current_no: + flat_queryset = queryset.values_list("id", flat=True)[:CLEAR_EXPIRED_TOKENS_BATCH_SIZE] + batch_length = flat_queryset.count() + queryset.model.objects.filter(id__in=list(flat_queryset)).delete() + logger.debug(f"{batch_length} tokens deleted, {current_no-batch_length} left") + queryset = queryset.model.objects.filter(query) + time.sleep(CLEAR_EXPIRED_TOKENS_BATCH_INTERVAL) + current_no = queryset.count() + + stop_no = queryset.model.objects.filter(query).count() + deleted = start_no - stop_no + return deleted + now = timezone.now() refresh_expire_at = None access_token_model = get_access_token_model() refresh_token_model = get_refresh_token_model() grant_model = get_grant_model() REFRESH_TOKEN_EXPIRE_SECONDS = oauth2_settings.REFRESH_TOKEN_EXPIRE_SECONDS + if REFRESH_TOKEN_EXPIRE_SECONDS: if not isinstance(REFRESH_TOKEN_EXPIRE_SECONDS, timedelta): try: @@ -636,31 +656,32 @@ def clear_expired(): raise ImproperlyConfigured(e) refresh_expire_at = now - REFRESH_TOKEN_EXPIRE_SECONDS - with transaction.atomic(): - if refresh_expire_at: - revoked = refresh_token_model.objects.filter( - revoked__lt=refresh_expire_at, - ) - expired = refresh_token_model.objects.filter( - access_token__expires__lt=refresh_expire_at, - ) + if refresh_expire_at: + revoked_query = models.Q(revoked__lt=refresh_expire_at) + revoked = refresh_token_model.objects.filter(revoked_query) + + revoked_deleted_no = batch_delete(revoked, revoked_query) + logger.info("%s Revoked refresh tokens deleted", revoked_deleted_no) + + expired_query = models.Q(access_token__expires__lt=refresh_expire_at) + expired = refresh_token_model.objects.filter(expired_query) - logger.info("%s Revoked refresh tokens to be deleted", revoked.count()) - logger.info("%s Expired refresh tokens to be deleted", expired.count()) + expired_deleted_no = batch_delete(expired, expired_query) + logger.info("%s Expired refresh tokens deleted", expired_deleted_no) + else: + logger.info("refresh_expire_at is %s. No refresh tokens deleted.", refresh_expire_at) - revoked.delete() - expired.delete() - else: - logger.info("refresh_expire_at is %s. No refresh tokens deleted.", refresh_expire_at) + access_token_query = models.Q(refresh_token__isnull=True, expires__lt=now) + access_tokens = access_token_model.objects.filter(access_token_query) - access_tokens = access_token_model.objects.filter(refresh_token__isnull=True, expires__lt=now) - grants = grant_model.objects.filter(expires__lt=now) + access_tokens_delete_no = batch_delete(access_tokens, access_token_query) + logger.info("%s Expired access tokens deleted", access_tokens_delete_no) - logger.info("%s Expired access tokens to be deleted", access_tokens.count()) - logger.info("%s Expired grant tokens to be deleted", grants.count()) + grants_query = models.Q(expires__lt=now) + grants = grant_model.objects.filter(grants_query) - access_tokens.delete() - grants.delete() + grants_deleted_no = batch_delete(grants, grants_query) + logger.info("%s Expired grant tokens deleted", grants_deleted_no) def redirect_to_uri_allowed(uri, allowed_uris): diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index 9a996b0c2..22e067716 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -101,6 +101,8 @@ # Whether to re-create OAuthlibCore on every request. # Should only be required in testing. "ALWAYS_RELOAD_OAUTHLIB_CORE": False, + "CLEAR_EXPIRED_TOKENS_BATCH_SIZE": 10000, + "CLEAR_EXPIRED_TOKENS_BATCH_INTERVAL": 0.1, } # List of settings that cannot be empty diff --git a/tests/settings.py b/tests/settings.py index bc7a55130..d2fbe6a56 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -156,3 +156,6 @@ OAUTH2_PROVIDER_APPLICATION_MODEL = "oauth2_provider.Application" OAUTH2_PROVIDER_REFRESH_TOKEN_MODEL = "oauth2_provider.RefreshToken" OAUTH2_PROVIDER_ID_TOKEN_MODEL = "oauth2_provider.IDToken" + +CLEAR_EXPIRED_TOKENS_BATCH_SIZE = 1 +CLEAR_EXPIRED_TOKENS_BATCH_INTERVAL = 0 From 2340bdf9ee4ec9ce2df6d3cd198db4ffbf8d3616 Mon Sep 17 00:00:00 2001 From: Dawid Wolski Date: Wed, 22 Dec 2021 18:09:11 +0100 Subject: [PATCH 2/3] CHANGELOG.md and AUTHORS Do not check for merge conflicts in AUTHORS file, because ======= in 2nd line triggers the error. --- .pre-commit-config.yaml | 1 + AUTHORS | 1 + CHANGELOG.md | 2 ++ 3 files changed, 4 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 177f2a25f..df18f7214 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,6 +10,7 @@ repos: - id: check-ast - id: trailing-whitespace - id: check-merge-conflict + exclude: AUTHORS - id: check-json - id: check-xml - id: check-yaml diff --git a/AUTHORS b/AUTHORS index deb0c7ce4..3d15e5310 100644 --- a/AUTHORS +++ b/AUTHORS @@ -24,6 +24,7 @@ Bas van Oostveen Dave Burkholder David Fischer David Smith +Dawid Wolski Diego Garcia Dulmandakh Sukhbaatar Dylan Giesler diff --git a/CHANGELOG.md b/CHANGELOG.md index dbf5eee2e..4a47d42b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 --> ## [Unreleased] +### Added +* #651 Batch expired token deletions in `cleartokens` management command ## [1.6.0] 2021-12-19 ### Added From caf5679fbc4095c4e666e55d7bbdeafab03ee46c Mon Sep 17 00:00:00 2001 From: Dawid Wolski Date: Mon, 27 Dec 2021 12:45:25 +0100 Subject: [PATCH 3/3] Issue with AUTHORS file fixed in 1.6.1 --- .pre-commit-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index df18f7214..177f2a25f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,7 +10,6 @@ repos: - id: check-ast - id: trailing-whitespace - id: check-merge-conflict - exclude: AUTHORS - id: check-json - id: check-xml - id: check-yaml