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

Add LIMIT statement to those queries needed during Pulp startup process. #6272

Merged
merged 1 commit into from
Feb 13, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions pulpcore/app/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@
STATIC_URL = "/assets/"
STATIC_ROOT = DEPLOY_ROOT / STATIC_URL.strip("/")

# begin compatilibity layer for DEFAULT_FILE_STORAGE
# begin compatibility layer for DEFAULT_FILE_STORAGE
# Remove on pulpcore=3.85 or pulpcore=4.0

# - What is this?
# We shouldnt use STORAGES or DEFAULT_FILE_STORAGE directly because those are
# We shouldn't use STORAGES or DEFAULT_FILE_STORAGE directly because those are
# mutually exclusive by django, which constraints users to use whatever we use.
# This is a hack/workaround to set Pulp's default while still enabling users to choose
# the legacy or the new storage setting.
Expand Down Expand Up @@ -295,7 +295,7 @@
# how long to protect ephemeral items in minutes
ORPHAN_PROTECTION_TIME = 24 * 60

# Custom cleaup intervals
# Custom cleanup intervals
# for the following, if set to 0, the corresponding cleanup task is disabled
UPLOAD_PROTECTION_TIME = 0
TASK_PROTECTION_TIME = 0
Expand Down Expand Up @@ -510,7 +510,7 @@ def otel_middleware_hook(settings):
post_hooks=otel_middleware_hook,
)

# begin compatilibity layer for DEFAULT_FILE_STORAGE
# begin compatibility layer for DEFAULT_FILE_STORAGE
# Remove on pulpcore=3.85 or pulpcore=4.0

# Ensures the cached property storage.backends uses the the right value
Expand Down Expand Up @@ -552,7 +552,9 @@ def otel_middleware_hook(settings):
with connection.cursor() as cursor:
for checksum in ALLOWED_CONTENT_CHECKSUMS:
# can't import Artifact here so use a direct db connection
cursor.execute(f"SELECT count(pulp_id) FROM core_artifact WHERE {checksum} IS NULL")
cursor.execute(
f"SELECT count(pulp_id) FROM core_artifact WHERE {checksum} IS NULL LIMIT 1"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't EXISTS be a bit simpler?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't EXISTS a subquery operator? https://www.postgresql.org/docs/current/functions-subquery.html#FUNCTIONS-SUBQUERY-EXISTS

How could we use it in this context?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, the django .exists() doesn't map to the SQL concept of EXISTS as I thought it did.

row = cursor.fetchone()
if row[0] > 0:
raise ImproperlyConfigured(
Expand All @@ -565,7 +567,7 @@ def otel_middleware_hook(settings):
for checksum in FORBIDDEN_CHECKSUMS:
# can't import Artifact here so use a direct db connection
cursor.execute(
f"SELECT count(pulp_id) FROM core_artifact WHERE {checksum} IS NOT NULL"
f"SELECT count(pulp_id) FROM core_artifact WHERE {checksum} IS NOT NULL LIMIT 1"
)
row = cursor.fetchone()
if row[0] > 0:
Expand All @@ -583,7 +585,7 @@ def otel_middleware_hook(settings):
cond = " AND ".join([f"{c} IS NULL" for c in ALLOWED_CONTENT_CHECKSUMS])
cursor.execute(
f"SELECT count(pulp_id) FROM core_remoteartifact WHERE {cond} AND "
f"pulp_id NOT IN ({no_checksum_query})"
f"pulp_id NOT IN ({no_checksum_query}) LIMIT 1"
)
row = cursor.fetchone()
if row[0] > 0:
Expand Down
Loading