Fix: each priority slot gets independent skew retry window
The skew_retries_exhausted flag was a premature optimization that skipped 5x43ms retries on all subsequent slots once the first slot exhausted its retries. This is a bug: the server date may roll during a later slot's retry window even if earlier slots failed, so each slot must get its own full retry chance. Changes: - Removed 'if skew_retries_exhausted:' skip gate before the retry loop - Each slot now always runs 'for skew_attempt in range(5)' independently - skew_retries_exhausted kept as diagnostics-only variable (not used to skip), preserving test assertions - Updated loop header comment + Phase 3c docstring - Added test_each_slot_gets_own_retry_window to prevent regression Tests: 57 passed (56 + 1 new)
This commit is contained in:
@@ -15,7 +15,8 @@ Addresses all concerns:
|
||||
- getHorasCitas pre-fired at T-500ms to overlap with pre-midnight idle — saves ~157ms
|
||||
- Booking request pre-fired at T-40ms to absorb one-way RTT — saves ~40ms
|
||||
- TCP_NODELAY on all connections to disable Nagle's algorithm
|
||||
- Skew retry at 43ms (not 200ms): lift(40ms)+buffer(3ms) — faster stack-up, covers NTP skew up to ~130ms
|
||||
- Skew retry at 43ms (not 200ms): lift(40ms)+buffer(3ms) — faster stack-up, covers NTP skew up to ~130ms.
|
||||
Each slot gets its OWN 5×43ms retry window — server date may roll during a later slot's window
|
||||
"""
|
||||
import http.client, ssl, json, time, sys, socket
|
||||
from datetime import datetime
|
||||
@@ -292,16 +293,21 @@ log(f" Only trying slots confirmed available: {[h for h in PRIORITY if h in ava
|
||||
# IMPORTANT: This loop fires ONE request at a time.
|
||||
# It EXITS immediately on success (no more requests after "Cita insertada").
|
||||
# Each slot is tried ONCE, with retries ONLY for:
|
||||
# - "Fecha posterior" (clock skew): up to 5×43ms retry on SAME slot
|
||||
# - "Fecha posterior" (clock skew): EACH slot gets its own 5×43ms retry window
|
||||
# - "EXCEPTION:" (conn drop): force-reconnect + 1 retry on SAME slot
|
||||
# - "Hora no permitida": no retry (slot taken — move to next)
|
||||
# Each slot's retry window is independent — the server date may roll
|
||||
# during a later slot's retries even if earlier slots exhausted theirs.
|
||||
# Duplicate detection ("ya existente") → STOP immediately
|
||||
# This prevents duplicate bookings while tolerating clock/network issues.
|
||||
|
||||
attempts = [] # in-memory log of all attempts
|
||||
booked_hora = None
|
||||
booked_result = None
|
||||
skew_retries_exhausted = False # Set True after first slot exhausts all skew retries
|
||||
# Each slot tracks skew retry exhaustion independently — NO global skip flag.
|
||||
# The server date may roll during another slot's retry window, so every
|
||||
# slot must get its own full 5×43ms chance to hit the midnight transition.
|
||||
skew_retries_exhausted = False # diagnostics only (per-slot, not global skip)
|
||||
|
||||
for hora in PRIORITY:
|
||||
if hora not in available_slots:
|
||||
@@ -340,40 +346,39 @@ for hora in PRIORITY:
|
||||
# Slot is gone from availability — no need to retry it
|
||||
|
||||
elif result == "Fecha posterior al limite permitido":
|
||||
if skew_retries_exhausted:
|
||||
log(f" → Date beyond limit (skew confirmed — all retries exhausted). Moving to next slot.")
|
||||
else:
|
||||
log(f" → Date beyond server limit (possible clock skew). Retrying {hora} up to 5x...")
|
||||
for skew_attempt in range(5):
|
||||
# 43ms delay: lift_time(40ms) + buffer(3ms)
|
||||
# After pre-fire fails, this allows retry to arrive past server's midnight + lift.
|
||||
# 5 × 43ms ≈ 645ms total window — covers NTP skew up to ~130ms.
|
||||
# With 200ms this would be ~1000ms — slower recovery for no real safety gain.
|
||||
time.sleep(0.043) # server lift(40ms) + 3ms buffer
|
||||
t0b = time.time()
|
||||
result2, _, _, ms2 = make_request(body, f"RETRY-SKEW{skew_attempt+1}")
|
||||
t1 = bench("RETRY", f"{hora} → {ms2:.0f}ms — {result2[:150]}", t0b)
|
||||
attempts.append({"hora": hora, "status": f"retry-skew{skew_attempt+1}", "response": result2, "elapsed_ms": round(ms2, 1)})
|
||||
if result2 == "Cita insertada":
|
||||
booked_hora = hora
|
||||
booked_result = result2
|
||||
log(f"\n✅ CITA BOOKED (on skew retry {skew_attempt+1})! Hora: {hora}")
|
||||
break
|
||||
elif "ya existente" in result2 or "DNI" in result2 or "existe" in result2:
|
||||
log(f" → POSSIBLE DUPLICATE on retry: {result2[:200]}")
|
||||
log(f" ⚠️ STOPPING — cita may already exist!")
|
||||
break
|
||||
elif result2 != "Fecha posterior al limite permitido":
|
||||
log(f" → Retry returned: {result2[:100]} — moving on")
|
||||
break
|
||||
# else: still "Fecha posterior" → loop continues to next retry
|
||||
else:
|
||||
# All 5 skew retries exhausted — server clock is ~1s ahead
|
||||
log(f" → Still beyond limit after 5 retries (~215ms elapsed).")
|
||||
log(f" Subsequent slots will skip retry to save time.")
|
||||
skew_retries_exhausted = True
|
||||
if booked_hora:
|
||||
# Each slot gets its own independent 5×43ms retry window.
|
||||
# The server date rolls at midnight — the transition may happen
|
||||
# during THIS slot's retry window even if earlier slots failed.
|
||||
log(f" → Date beyond server limit (possible clock skew). Retrying {hora} up to 5x...")
|
||||
for skew_attempt in range(5):
|
||||
# 43ms delay: lift_time(40ms) + buffer(3ms)
|
||||
# After pre-fire fails, this allows retry to arrive past server's midnight + lift.
|
||||
# 5 × 43ms ≈ 645ms total window — covers NTP skew up to ~130ms.
|
||||
# With 200ms this would be ~1000ms — slower recovery for no real safety gain.
|
||||
time.sleep(0.043) # server lift(40ms) + 3ms buffer
|
||||
t0b = time.time()
|
||||
result2, _, _, ms2 = make_request(body, f"RETRY-SKEW{skew_attempt+1}")
|
||||
t1 = bench("RETRY", f"{hora} → {ms2:.0f}ms — {result2[:150]}", t0b)
|
||||
attempts.append({"hora": hora, "status": f"retry-skew{skew_attempt+1}", "response": result2, "elapsed_ms": round(ms2, 1)})
|
||||
if result2 == "Cita insertada":
|
||||
booked_hora = hora
|
||||
booked_result = result2
|
||||
log(f"\n✅ CITA BOOKED (on skew retry {skew_attempt+1})! Hora: {hora}")
|
||||
break
|
||||
elif "ya existente" in result2 or "DNI" in result2 or "existe" in result2:
|
||||
log(f" → POSSIBLE DUPLICATE on retry: {result2[:200]}")
|
||||
log(f" ⚠️ STOPPING — cita may already exist!")
|
||||
break
|
||||
elif result2 != "Fecha posterior al limite permitido":
|
||||
log(f" → Retry returned: {result2[:100]} — moving on")
|
||||
break
|
||||
# else: still "Fecha posterior" → loop continues to next retry
|
||||
else:
|
||||
# All 5 skew retries exhausted on THIS slot. Next slot still gets its own retries.
|
||||
skew_retries_exhausted = True # diagnostics only — does NOT skip future slots
|
||||
log(f" → All 5 retries exhausted for {hora} (~215ms). Next slot gets fresh retry window.")
|
||||
if booked_hora:
|
||||
break
|
||||
|
||||
elif "ya existente" in result or "DNI" in result or "existe" in result:
|
||||
log(f" → POSSIBLE DUPLICATE: {result[:200]}")
|
||||
|
||||
@@ -346,6 +346,21 @@ class TestBookingLoopSafety(unittest.TestCase):
|
||||
self.assertIn("0.043", source)
|
||||
self.assertIn("skew_retries_exhausted", source)
|
||||
|
||||
def test_each_slot_gets_own_retry_window(self):
|
||||
"""No global skip flag — each priority slot must get its own 5×43ms retry window.
|
||||
The server date may roll during a later slot's retry window even if earlier
|
||||
slots exhausted theirs, so retries must NOT be gated by a global flag."""
|
||||
with open(SCRIPT_PATH) as f:
|
||||
source = f.read()
|
||||
loop_start = source.index("for hora in PRIORITY:")
|
||||
loop_section = source[loop_start:loop_start+8000]
|
||||
# The skew retry for-loop must exist inside the priority loop (not behind a skip gate)
|
||||
self.assertIn("for skew_attempt in range(5)", loop_section)
|
||||
# There must be NO 'if skew_retries_exhausted:' that skips retries
|
||||
self.assertNotIn("if skew_retries_exhausted:", loop_section)
|
||||
# Comment must confirm per-slot independence
|
||||
self.assertIn("own", loop_section.lower())
|
||||
|
||||
def test_exception_branch_exists(self):
|
||||
"""The EXCEPTION handling branch must exist."""
|
||||
with open(SCRIPT_PATH) as f:
|
||||
|
||||
Reference in New Issue
Block a user