[dkropachev]

Summary

Synchronous /localnodes polling previously opened a fresh TCP connection for every fetch because urllib forced Connection: close. Retain a manager-owned HTTP/1 pool with up to 200 idle connections, one per endpoint, and drain response bodies before returning connections to the pool, including non-2xx responses.

Keep urllib's proxy and redirect handling, existing TLS settings, and the total discovery deadline across DNS resolution, address fallback, and response reads. A discovery GET can retry once when a reused connection closes before response headers; both attempts share the same deadline.

Manager shutdown cancels active leases, interrupts sockets, and prevents stalled DNS workers from connecting later. Restart admits fresh requests, and failed initialization or thread/SDK construction closes discovery resources.

Fixes #160. Follow-up to closed #10; data-request host-pool sizing is handled separately in #161.

[qodo-scylladb]

Code Review by Qodo

🐞 Bugs (1)

---

🔴 High

1. Restart can lose background refreshes 🐞 Bug

Description

SyncLiveNodesManager.start() returns when the old refresh thread is still alive, even if stop()
has already set that thread's stop event. If start() runs while stop() is joining the thread,
the pool reopens and manual discovery works, but the old thread exits without a replacement to
perform future background refreshes.

Code

alternator/core/live_nodes.py[R404-405]

+                if self._refresh_thread is not None and self._refresh_thread.is_alive():
+                    return

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A start during the stop join window reopens discovery but returns because the stopping refresh thread is still alive; no background thread replaces it.
## Fix Focus Areas
- alternator/core/live_nodes.py[397-452]
- alternator/core/live_nodes.py[543-558]
- tests/unit/test_sync_discovery_pool.py[285-341]
## Recommended Fix
When the existing thread is alive but its stop event is set, arrange for a replacement to start after that thread exits, without replacing it while it is still running. Extend the overlapping stop/start test to verify that background refresh resumes, not only that manual refresh succeeds.

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

Dismiss ↗ | View ↗

---

---

Resolved findings

1. Dripping replies fail the deadline test ✗ Dismissed

Description

test_sync_discovery_deadline_interrupts_a_dripping_response() now requires the server to observe a
disconnect, but SyncConnectionPool.open() reads a successful body in one call and checks the
absolute deadline only after that call returns. With the test's 20 ms byte intervals and 80 ms
deadline, each socket read can receive data before its inactivity timeout, allowing the entire body
to arrive before the client closes the connection.

Code

tests/unit/test_sync_discovery_pool.py[507]

+        assert disconnected.wait(timeout=1)

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly asserted peer disconnect does not occur at the total discovery deadline because successful response reads can continue receiving intermittent bytes until the body is complete.
## Fix Focus Areas
- alternator/_sync_http_pool.py[185-216]
- tests/unit/test_sync_discovery_pool.py[471-507]
## Recommended Fix
Read successful bodies incrementally, checking the absolute deadline and updating the socket timeout between reads so a dripping response is interrupted before completion. Keep the peer-disconnect assertion as a regression check.

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

---

2. Shutdown cannot interrupt TCP connects ✓ Resolved

Description

SyncConnectionPool._acquire() records a new connection before its socket exists, while
_create_connection_with_deadline() keeps the socket local until connect() completes. If shutdown
or timeout occurs during a stalled TCP connect, _interrupt() has no socket to shut down, so the
discovery worker remains blocked until the connect timeout.

Code

alternator/_sync_http_pool.py[126]

+            self._active[connection] = (cancelled, connection.sock)

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new lease records `connection.sock` before TCP connection starts. During a blocking connect, the socket exists only as a local variable, so cancellation cannot interrupt it.

## Fix Focus Areas
- alternator/_sync_http_pool.py[73-81]
- alternator/_sync_http_pool.py[120-127]
- alternator/_http.py[78-101]

## Recommended Fix
Make the connecting socket available to its active lease before calling `connect()`, and remove or close it on failure. Ensure both timeout cancellation and pool shutdown can shut down that socket while the connect is pending.

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

---

3. Discovery can stay off after an overlapping stop and start ✓ Resolved

Description

SyncLiveNodesManager.stop() releases _lifecycle_lock before calling self._http_fetch.close(),
while start() reopens the pool and starts a refresh thread under that lock. If start() runs in
that gap, the older stop closes the pool beneath the new thread; subsequent pool.begin() calls
return already-cancelled tokens, so refreshes return no nodes until another start().

Code

alternator/core/live_nodes.py[R428-429]

+        if isinstance(self._http_fetch, SyncNodeFetcher):
+            self._http_fetch.close()

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SyncLiveNodesManager.stop()` closes the fetcher after releasing `_lifecycle_lock`. A concurrent `start()` can reopen the pool and start a new refresh thread before that older stop closes the pool again, leaving discovery inactive until another `start()`.

## Fix Focus Areas
- alternator/core/live_nodes.py[397-441]

## Recommended Fix
Move the `close()` call into the `with self._lifecycle_lock:` block in `stop()`, immediately after the events are set, so pool closure is ordered with `start()`'s `reopen()`. Keep the thread join outside the lock.

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

---

4. Failed helper startup leaves a socket open ✓ Resolved

Description

SyncLiveNodesManager.start() reopens the fetcher before calling refresh_thread.start(), but does
not close the pool if thread creation raises. A public Helper.start() or Helper.client() call
takes this path without the construction cleanup used by _create_and_start_manager(), so an idle
connection from initial discovery remains open until explicit cleanup or finalization.

Code

alternator/core/live_nodes.py[R403-404]

+            if isinstance(self._http_fetch, SyncNodeFetcher):
+                self._http_fetch.reopen()

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Public helper startup can leave the discovery pool open if starting its refresh thread raises.
## Fix Focus Areas
- alternator/core/live_nodes.py[397-417]
- alternator/client.py[588-614]
## Recommended Fix
Clean up the reopened pool and failed thread state when refresh-thread startup raises, including when start is called through public helper methods.

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

---

5. Large error bodies exhaust discovery memory ✓ Resolved

Description

SyncConnectionPool.open() unconditionally reads the entire response into a bytes object before
urllib processes its status. A server returning a very large non-2xx body now makes discovery buffer
content it never uses, rather than rejecting the response after its headers.

Code

alternator/_sync_http_pool.py[R205-207]

+                with response:
+                    body = response.read()
+                self.remaining(deadline, cancelled)

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The pooled transport buffers arbitrarily large error bodies before urllib rejects their status.
## Fix Focus Areas
- alternator/_sync_http_pool.py[200-214]
## Recommended Fix
Drain non-2xx responses only up to a defined limit; when the limit is exceeded, close rather than reuse the connection and return enough status metadata for urllib to reject the response.

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

---

---

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

---

Powered by Qodo

[dkropachev]

In reply to

Discovery can stay off after an overlapping stop and start

Fetcher closure now runs under the lifecycle lock, ordering it with restart; the thread join remains outside the lock.

Failed helper startup leaves a socket open

Refresh-thread construction or startup failure now closes the reopened fetcher and clears the failed thread state, including startup through Helper.

Large error bodies exhaust discovery memory

Non-2xx response bodies are drained in bounded chunks without retaining them, preserving status and headers for urllib and allowing reuse after a complete drain.

[dkropachev]

In reply to

Shutdown cannot interrupt TCP connects

Each TCP candidate is now exposed through the owning connection before connect starts, so pool shutdown and deadline cancellation can interrupt it. Failed candidates are closed and cleared. Regression tests cover both cancellation paths for HTTP and HTTPS.

[qodo-scylladb]

🔴 High

2. Restart can lose background refreshes 🐞 Bug ≡ Correctness

SyncLiveNodesManager.start() returns when the old refresh thread is still alive, even if stop()
has already set that thread's stop event. If start() runs while stop() is joining the thread,
the pool reopens and manual discovery works, but the old thread exits without a replacement to
perform future background refreshes.

Agent Prompt

## Issue description
A start during the stop join window reopens discovery but returns because the stopping refresh thread is still alive; no background thread replaces it.
## Fix Focus Areas
- alternator/core/live_nodes.py[397-452]
- alternator/core/live_nodes.py[543-558]
- tests/unit/test_sync_discovery_pool.py[285-341]
## Recommended Fix
When the existing thread is alive but its stop event is set, arrange for a replacement to start after that thread exits, without replacing it while it is still running. Extend the overlapping stop/start test to verify that background refresh resumes, not only that manual refresh succeeds.

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

Dismiss ↗ | View ↗