fix(vec_dbs): qdrant client explicit timeout + transient backoff retry - #2404

🤖 Open Code Review

Target: PR #2404
Task: 86cdf9eb52bea840
Base: main
Head: upstream-pr/qdrant-timeout-backoff

🔍 OpenCodeReview found 4 issue(s) in this PR.

1. src/memos/vec_dbs/qdrant.py (L151-L156)

Dead code: raise last after the loop is unreachable. The loop always exits via return fn() on success, or raise (line if not transient or attempt == 2: raise) on the last attempt. Since attempt == 2 is always true on the final iteration, execution never falls through the loop body to raise last. This is harmless today but may mask a correctness bug if the retry count or exit condition is refactored later — the fallback raise would silently execute raise None, producing an unhelpful TypeError: exceptions must derive from BaseException.

Suggested fix: remove the unreachable raise last line, or restructure the loop to make the sentinel meaningful (e.g., use for/else).

                if not transient or attempt == 2:
                    raise
                wait = (2 if attempt == 0 else 5) + random.uniform(0, 1.5)
                logger.info(f"[VecDB] {desc} transient error, retry in {wait:.1f}s: {e}")
                time.sleep(wait)
        raise last

                if not transient or attempt == 2:
                    raise
                wait = (2 if attempt == 0 else 5) + random.uniform(0, 1.5)
                logger.info(f"[VecDB] {desc} transient error, retry in {wait:.1f}s: {e}")
                time.sleep(wait)
        # `raise last` removed — the loop always exits via return or raise above

2. src/memos/vec_dbs/qdrant.py (L144-L150)

Over-broad transient detection: "connect" in msg matches any error message containing that substring, including non-transient failures such as "failed to connect due to authentication error", "disconnect caused by invalid API key", or "incorrect connection string". These errors would be silently retried twice, adding up to 13+ seconds of delay before ultimately failing, and obscuring the root cause in logs.

Prefer matching a more specific token like "connection refused" or "connection reset", or — better — catching typed exceptions from qdrant_client (e.g., qdrant_client.http.exceptions.UnexpectedResponse, grpc.RpcError) instead of relying on string matching.

                transient = (
                    "timed out" in msg
                    or "timeout" in msg
                    or "connect" in msg
                    or "503" in msg
                    or "unavailable" in msg
                )

                transient = (
                    "timed out" in msg
                    or "timeout" in msg
                    or "connection refused" in msg
                    or "connection reset" in msg
                    or "503" in msg
                    or "unavailable" in msg
                )

3. src/memos/vec_dbs/qdrant.py (L134-L135)

Imports inside a hot-path method: import random and import time are placed inside _call_with_backoff, which is called on every search and get_by_ids invocation. Python caches module imports in sys.modules, so the per-call cost is a dict lookup rather than a full load — but the pattern is inconsistent with the module-level import os added in the same PR and makes the dependencies of this function non-obvious. Move both imports to the module-level import block.

        import random
        import time

# Move to module-level imports at the top of the file:
import os
import random
import time

4. src/memos/vec_dbs/qdrant.py (L154)

Eager f-string in logger call: Passing an f-string to logger.info(...) forces string interpolation on every retry, even when the INFO log level is disabled. Use %-style lazy formatting so the string is only built when the message will actually be emitted.

                logger.info(f"[VecDB] {desc} transient error, retry in {wait:.1f}s: {e}")

                logger.info("[VecDB] %s transient error, retry in %.1fs: %s", desc, wait, e)

Generated by cloud-assistant via Open Code Review.

✅ Automated Test Results: PASSED

All tests passed (2/2 executed). memos_python_core/changed-python-source: 2/2. Duration: 8s [advisory, non-gating] AI-generated tests on branch test/auto-gen-86cdf9eb52bea840-20260922093054: 79/79 passed — these do NOT affect the PR verdict; review the branch manually.

Branch: upstream-pr/qdrant-timeout-backoff