fix(dolt): A2 review — subprocess timeouts, stronger initial gate, HASHOF
Addresses the A2 review:
1. Every dolt subprocess is now bounded by a hard timeout
(dolt_command_timeout_seconds, default 600s); on expiry the process is killed
and DoltError raised — a hung pull/sql can no longer pin the import
connection and advisory lock indefinitely. Tested (timeout + non-zero exit).
2. Initial-load validate is stronger: besides zero-future, an initial load now
requires a real forward horizon (>= 21d, under the ~35d observed on the
clone) AND universe coverage >= 50% (a broken symbol join can't seed a hollow
calendar). Subsequent runs keep the 50% collapse gate.
3. Revision uses DOLT_HASHOF('HEAD') — formally HEAD, not dolt_log-by-timestamp.
4. Free-disk floor raised 2 GB -> 5 GB (safe headroom over the ~1.7 GB clone).
Full suite 702 passed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+33
-11
@@ -23,12 +23,19 @@ from pathlib import Path
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Default subprocess timeout. A hung `dolt pull`/`sql` would otherwise pin the
|
||||
# import's connection and its advisory lock indefinitely, so every call is
|
||||
# bounded; callers may override per operation.
|
||||
DEFAULT_TIMEOUT = 600.0
|
||||
|
||||
|
||||
class DoltError(RuntimeError):
|
||||
"""A dolt subprocess exited non-zero."""
|
||||
"""A dolt subprocess failed, timed out, or exited non-zero."""
|
||||
|
||||
|
||||
async def _run(binary: str, args: list[str], *, cwd: Path) -> str:
|
||||
async def _run(
|
||||
binary: str, args: list[str], *, cwd: Path, timeout: float = DEFAULT_TIMEOUT
|
||||
) -> str:
|
||||
proc = await asyncio.create_subprocess_exec(
|
||||
binary,
|
||||
*args,
|
||||
@@ -36,7 +43,15 @@ async def _run(binary: str, args: list[str], *, cwd: Path) -> str:
|
||||
stdout=asyncio.subprocess.PIPE,
|
||||
stderr=asyncio.subprocess.PIPE,
|
||||
)
|
||||
stdout, stderr = await proc.communicate()
|
||||
try:
|
||||
stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=timeout)
|
||||
except asyncio.TimeoutError:
|
||||
proc.kill()
|
||||
try:
|
||||
await proc.wait()
|
||||
except ProcessLookupError:
|
||||
pass
|
||||
raise DoltError(f"dolt {args[0] if args else ''} timed out after {timeout:.0f}s")
|
||||
if proc.returncode != 0:
|
||||
raise DoltError(
|
||||
f"dolt {' '.join(args)} failed ({proc.returncode}): "
|
||||
@@ -60,24 +75,31 @@ def ensure_free_disk(path: Path, min_free_gb: float) -> None:
|
||||
)
|
||||
|
||||
|
||||
async def pull(repo_dir: Path, *, binary: str) -> None:
|
||||
async def pull(repo_dir: Path, *, binary: str, timeout: float = DEFAULT_TIMEOUT) -> None:
|
||||
"""`dolt pull` the persistent clone to the latest upstream revision."""
|
||||
await _run(binary, ["pull"], cwd=repo_dir)
|
||||
await _run(binary, ["pull"], cwd=repo_dir, timeout=timeout)
|
||||
|
||||
|
||||
async def current_commit(repo_dir: Path, *, binary: str) -> str:
|
||||
"""The HEAD commit hash of the clone — used as the import revision."""
|
||||
async def current_commit(
|
||||
repo_dir: Path, *, binary: str, timeout: float = DEFAULT_TIMEOUT
|
||||
) -> str:
|
||||
"""The HEAD commit hash of the clone — used as the import revision.
|
||||
|
||||
Uses ``DOLT_HASHOF('HEAD')`` (which formally identifies HEAD) rather than
|
||||
ordering ``dolt_log`` by timestamp."""
|
||||
rows = await query_csv(
|
||||
repo_dir, "SELECT commit_hash FROM dolt_log ORDER BY date DESC LIMIT 1", binary=binary
|
||||
repo_dir, "SELECT DOLT_HASHOF('HEAD') AS commit_hash", binary=binary, timeout=timeout
|
||||
)
|
||||
if not rows or not rows[0].get("commit_hash"):
|
||||
raise DoltError("could not read HEAD commit hash from dolt_log")
|
||||
raise DoltError("could not read HEAD commit hash")
|
||||
return rows[0]["commit_hash"]
|
||||
|
||||
|
||||
async def query_csv(repo_dir: Path, sql: str, *, binary: str) -> list[dict[str, str]]:
|
||||
async def query_csv(
|
||||
repo_dir: Path, sql: str, *, binary: str, timeout: float = DEFAULT_TIMEOUT
|
||||
) -> list[dict[str, str]]:
|
||||
"""Run a read query and parse the CSV result into a list of dict rows."""
|
||||
out = await _run(binary, ["sql", "-q", sql, "-r", "csv"], cwd=repo_dir)
|
||||
out = await _run(binary, ["sql", "-q", sql, "-r", "csv"], cwd=repo_dir, timeout=timeout)
|
||||
if not out.strip():
|
||||
return []
|
||||
return list(csv.DictReader(io.StringIO(out)))
|
||||
|
||||
@@ -49,6 +49,14 @@ MAX_LEAD_DAYS = 14
|
||||
# currently-loaded forward calendar (guards the destructive re-insert against a
|
||||
# partial parse / symbol-mapping regression).
|
||||
MIN_FUTURE_RATIO = 0.5
|
||||
# Initial-load gates (when nothing is loaded yet — the ratio gate has no baseline).
|
||||
# The source publishes a forward calendar; require a real horizon, not one stray
|
||||
# future row. 21 days is a conservative floor under the ~35d horizon observed on
|
||||
# the live clone.
|
||||
MIN_FORWARD_HORIZON_DAYS = 21
|
||||
# ...and require the symbol join to reach most of the tracked universe, so a
|
||||
# broken/normalization-dropped join can't seed a hollow calendar.
|
||||
MIN_INITIAL_COVERAGE = 0.5
|
||||
|
||||
_CAL_SQL = (
|
||||
"SELECT act_symbol, `date`, `when` FROM earnings_calendar "
|
||||
@@ -96,16 +104,24 @@ class DoltEarningsImporter:
|
||||
# -- SourceImporter protocol -------------------------------------------
|
||||
|
||||
async def detect_revision(self, db) -> str | None:
|
||||
timeout = settings.dolt_command_timeout_seconds
|
||||
if self.do_pull:
|
||||
dolt_client.ensure_free_disk(self.repo_dir, settings.dolt_min_free_disk_gb)
|
||||
await self._dolt.pull(self.repo_dir, binary=self.binary)
|
||||
return await self._dolt.current_commit(self.repo_dir, binary=self.binary)
|
||||
await self._dolt.pull(self.repo_dir, binary=self.binary, timeout=timeout)
|
||||
return await self._dolt.current_commit(
|
||||
self.repo_dir, binary=self.binary, timeout=timeout
|
||||
)
|
||||
|
||||
async def stage(self, db) -> StagedEarnings:
|
||||
universe = await self._load_universe(db) # {normalised symbol: ticker_id}
|
||||
|
||||
cal_raw = await self._dolt.query_csv(self.repo_dir, _CAL_SQL, binary=self.binary)
|
||||
hist_raw = await self._dolt.query_csv(self.repo_dir, _HIST_SQL, binary=self.binary)
|
||||
timeout = settings.dolt_command_timeout_seconds
|
||||
cal_raw = await self._dolt.query_csv(
|
||||
self.repo_dir, _CAL_SQL, binary=self.binary, timeout=timeout
|
||||
)
|
||||
hist_raw = await self._dolt.query_csv(
|
||||
self.repo_dir, _HIST_SQL, binary=self.binary, timeout=timeout
|
||||
)
|
||||
_require_columns(cal_raw, {"act_symbol", "date", "when"}, "earnings_calendar")
|
||||
_require_columns(
|
||||
hist_raw, {"act_symbol", "period_end_date", "reported", "estimate"}, "eps_history"
|
||||
@@ -164,13 +180,36 @@ class DoltEarningsImporter:
|
||||
)
|
||||
|
||||
async def validate(self, db, staged: StagedEarnings) -> ValidationResult:
|
||||
# Promote deletes+reinserts the forward calendar, so this gate is
|
||||
# fail-closed. The forward calendar is the project's acceptance gate.
|
||||
messages: list[str] = []
|
||||
current_future = await self._current_future_count(db)
|
||||
universe_size = int(staged.stats.get("universe_size", 0) or 0)
|
||||
coverage = (
|
||||
staged.stats.get("symbols_with_calendar", 0) / universe_size
|
||||
if universe_size
|
||||
else 0.0
|
||||
)
|
||||
horizon_days = (
|
||||
(staged.max_announce_date - self.today).days if staged.max_announce_date else 0
|
||||
)
|
||||
|
||||
# Fail-closed forward-calendar protection (promote deletes+reinserts it).
|
||||
if staged.future_count == 0:
|
||||
messages.append("no future-dated earnings rows staged")
|
||||
current_future = await self._current_future_count(db)
|
||||
if current_future > 0 and staged.future_count < current_future * MIN_FUTURE_RATIO:
|
||||
elif current_future == 0:
|
||||
# Initial load: no baseline for the ratio gate, so require a real
|
||||
# forward horizon and broad universe coverage instead of one stray row.
|
||||
if horizon_days < MIN_FORWARD_HORIZON_DAYS:
|
||||
messages.append(
|
||||
f"forward horizon only {horizon_days}d < {MIN_FORWARD_HORIZON_DAYS}d "
|
||||
"on initial load"
|
||||
)
|
||||
if coverage < MIN_INITIAL_COVERAGE:
|
||||
messages.append(
|
||||
f"initial universe coverage {coverage:.0%} "
|
||||
f"< {MIN_INITIAL_COVERAGE:.0%} — symbol join likely broken"
|
||||
)
|
||||
elif staged.future_count < current_future * MIN_FUTURE_RATIO:
|
||||
messages.append(
|
||||
f"forward calendar collapsed: staged {staged.future_count} future rows "
|
||||
f"< {MIN_FUTURE_RATIO:.0%} of current {current_future}"
|
||||
@@ -185,6 +224,8 @@ class DoltEarningsImporter:
|
||||
"staged_rows": len(staged.rows),
|
||||
"future_rows": staged.future_count,
|
||||
"current_future_rows": current_future,
|
||||
"forward_horizon_days": horizon_days,
|
||||
"universe_coverage": round(coverage, 3),
|
||||
}
|
||||
return ValidationResult(
|
||||
ok=not messages,
|
||||
|
||||
Reference in New Issue
Block a user