Skip to content

tests: retry client-routes POST on transient 5xx after decommission #1019

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.

Sign up for GitHub

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

Open
mykaul wants to merge 1 commit into
base: master
Choose a base branch
from
Open

tests: retry client-routes POST on transient 5xx after decommission #1019

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 31 additions & 11 deletions tests/integration/standard/test_client_routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import uuid

import json as _json
import urllib.error
import urllib.request

from cassandra.cluster import Cluster
Expand Down Expand Up @@ -232,17 +233,36 @@ def post_client_routes(contact_point, routes):
url = "http://%s:10000/v2/client-routes" % contact_point
log.info("Posting %d routes to %s", len(payload), url)
data = _json.dumps(payload).encode("utf-8")
req = urllib.request.Request(
url,
data=data,
headers={
"Content-Type": "application/json",
"Accept": "application/json",
},
method="POST",
)
response = urllib.request.urlopen(req)
log.info("Routes posted successfully (status %d)", response.status)

# A node can still be settling gossip/topology right after a
# decommission/bootstrap, making the REST API answer with a transient
# 500 for a brief window. Retry bounded 5xx responses; anything else
# (4xx, connection errors) is a real bug and should raise immediately.
max_attempts = 5
for attempt in range(1, max_attempts + 1):
req = urllib.request.Request(
url,
data=data,
headers={
"Content-Type": "application/json",
"Accept": "application/json",
},
method="POST",
)
try:
response = urllib.request.urlopen(req)
log.info("Routes posted successfully (status %d)", response.status)
return
Comment on lines +253 to +255

@qodo-scylladb qodo-scylladb Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Repeated route posts leak http responses 🐞 Bug ☼ Reliability

The success path stores the object returned by urlopen() but returns without closing it, while the
new HTTP-error path reads the error body without closing that response either. Calls made repeatedly
during route updates can consequently retain HTTP connections or file descriptors until garbage
collection, making the test process less reliable under repeated retries and updates.
Agent Prompt 
Issue description
`post_client_routes()` does not explicitly close successful responses or `HTTPError` response objects, and the new retry loop can create several response objects per invocation.

Fix Focus Areas
- tests/integration/standard/test_client_routes.py[253-265]

Recommended Fix
Use a context manager around each successful `urlopen()` response and close `HTTPError` objects after reading their bodies, preferably with cleanup in a `finally` block so both retry and terminal-error paths release resources.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

except urllib.error.HTTPError as e:
body = e.read().decode("utf-8", "replace")
if e.code >= 500 and attempt < max_attempts:

@qodo-scylladb qodo-scylladb Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Non-5xx errors receive unnecessary retries 🐞 Bug ≡ Correctness

The retry predicate accepts every HTTP error code at or above 500 instead of only codes in the
500–599 range. A non-standard status such as 600 therefore waits and retries four times before
failing, contrary to the helper's documented fail-fast behavior for errors outside 5xx.
Agent Prompt 
Issue description
The retry condition uses `e.code >= 500`, so HTTP error codes above 599 are retried even though the helper is intended to retry only 5xx responses.

Fix Focus Areas
- tests/integration/standard/test_client_routes.py[258-258]

Recommended Fix
Change the predicate to `500 <= e.code < 600` so only standard 5xx responses are retried; preserve immediate failure for 4xx and other status codes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

log.warning(
"POST %s -> HTTP %d (attempt %d/%d), retrying: %s",
url, e.code, attempt, max_attempts, body)
time.sleep(1)
continue
log.error("POST %s -> HTTP %d: %s", url, e.code, body)
raise


def get_host_ids_from_cluster(session):
Expand Down
Loading