Skip to content

Commit 115efd6

Browse files
committed
fix(load): submit a managed load as a job and poll, don't hold a request open
A load's duration scales with the data, so waiting for one inside a single HTTP request makes it depend on every layer between the client and the engine staying up for the whole thing -- CDN, gateway, and this client's own socket read deadline. Any one of them ending the request loses the load: the server marks it `abandoned` and DISCARDS work it had already done, while the table's write lock is still held against the retry that follows. The retry then collides with that lock (409 RESOURCE_LOCKED) and the two can alternate indefinitely without the load ever completing. Seen in production on a table whose load runs past five minutes: the same load acquired, was abandoned at ~4m45s, and re-acquired, three times in a row, while the caller retried 409s for twenty-five minutes. Small tables on the same connection succeeded in under two seconds throughout, which is what ruled out the lock itself being at fault. The server already offers the fix and recommends it for this exact case -- "run the load as a background job and return a job ID to poll instead of blocking until it finishes. Recommended for large uploads, which can take longer than an HTTP request should stay open." So loads now go out with `async` set. `async_after_ms` keeps the common case unchanged. The server answers 200 with the result when the load finishes inside the window and only defers to a job when it does not, so nothing pays for polling that did not need it -- dlt's bookkeeping tables settle in under a second and would otherwise gain a submit-then-poll round trip each. The polling budget is the load's own, deliberately not the 300s that queries and results share: reusing that is what put a five-minute ceiling on loads to begin with. Bounded rather than unbounded, so a wedged job still surfaces as a timeout. `_poll_job` already existed for the index path, which has had this shape all along; this reuses it rather than adding a second poller. Worth noting for `append`: it is the one non-idempotent mode, and the reason a lost response is dangerous there is that "did it land?" becomes unanswerable. A job id outlives the request that created it, so that ambiguity is gone -- the retry rule is unchanged here, but it no longer rests on the response surviving. Verified against a live deployment: the server accepts the new fields, a fast load still returns inline, and no polling happens when it does. The deferred path is covered by unit tests -- reproducing it live needs a load slow enough to exceed the window, which the reporting workload produces and a synthetic 300k-row load (0.8s) does not.
1 parent f98c313 commit 115efd6

3 files changed

Lines changed: 233 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- fix(load): submit managed loads as a job and poll, instead of holding one request open
13+
1014

1115
## [0.12.0] - 2026-08-11
1216

hotdata_framework/client.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from hotdata.models.database_default_table_decl import DatabaseDefaultTableDecl
2525
from hotdata.models.index_info_response import IndexInfoResponse
2626
from hotdata.models.job_status_response import JobStatusResponse
27+
from hotdata.models.load_managed_table_response import LoadManagedTableResponse
2728
from hotdata.models.load_managed_table_request import LoadManagedTableRequest
2829
from hotdata.models.query_request import QueryRequest
2930
from hotdata.models.query_response import QueryResponse
@@ -81,6 +82,19 @@
8182
# Jobs have no "cancelled" state; "partially_succeeded" carries an error_message.
8283
_JOB_TERMINAL = frozenset({"succeeded", "partially_succeeded", "failed"})
8384

85+
# How long a load may finish INLINE before the server hands back a job instead.
86+
# Small enough that a slow load stops holding a request open, large enough that
87+
# the overwhelming majority never become jobs at all: dlt's bookkeeping tables
88+
# (`_dlt_version`, `_dlt_loads`, `_dlt_pipeline_state`) settle in under a second,
89+
# and paying a submit-then-poll round trip for those would be a regression.
90+
_LOAD_INLINE_WAIT_MS = 10_000
91+
92+
# A load's own polling budget. Deliberately NOT the 300s used for queries and
93+
# results: a load is the one operation here whose duration scales with the data,
94+
# and reusing the query budget is what put a five-minute ceiling on it in the
95+
# first place. Bounded rather than unbounded so a wedged job still surfaces.
96+
_LOAD_JOB_TIMEOUT_S = 3600.0
97+
8498

8599
@dataclass(frozen=True)
86100
class ResultSummary:
@@ -418,10 +432,25 @@ def load_managed_table(
418432
else:
419433
assert file is not None
420434
resolved_upload_id = self.upload_parquet(file)
435+
# ASKED FOR AS A JOB, not as a held-open request. A load's duration scales
436+
# with the data, and a single request that must survive minutes has to
437+
# survive every layer between here and the engine -- CDN, gateway, socket
438+
# read timeout -- any one of which ends it. When it ends, the server logs
439+
# the load `abandoned` and DISCARDS work it had already done, while the
440+
# table's write lock is still held against the retry that follows; the
441+
# retry then collides with it (409 RESOURCE_LOCKED) and the pair can spin
442+
# indefinitely without the load ever completing. Observed in production on
443+
# a table whose load runs past five minutes.
444+
#
445+
# `async_after_ms` keeps the common case unchanged: the server answers 200
446+
# with the result if it finishes inside the window, and only falls back to
447+
# a job when it does not. So nothing pays for polling that did not need it.
421448
request = LoadManagedTableRequest(
422449
mode=mode,
423450
upload_id=resolved_upload_id,
424451
key=key,
452+
var_async=True,
453+
async_after_ms=_LOAD_INLINE_WAIT_MS,
425454
)
426455
try:
427456
loaded = self.connections().load_managed_table(
@@ -432,6 +461,8 @@ def load_managed_table(
432461
)
433462
except ApiException as e:
434463
raise RuntimeError(api_error_message(e)) from e
464+
if isinstance(loaded, SubmitJobResponse):
465+
loaded = self._load_response_from_job(loaded.id)
435466
return LoadManagedTableResult(
436467
connection_id=loaded.connection_id,
437468
schema_name=loaded.schema_name,
@@ -893,6 +924,33 @@ def _poll_job(
893924
f"Job {job_id} did not finish within {timeout_s}s (last status: {last_status})"
894925
)
895926

927+
def _load_response_from_job(self, job_id: str) -> LoadManagedTableResponse:
928+
"""The result of a load the server chose to run as a job.
929+
930+
Polling replaces waiting on the request, so the outcome is read from
931+
durable state rather than from a connection that has to stay alive. That
932+
also removes the ambiguity `append` was made non-retryable for: a lost
933+
response no longer leaves "did it land?" unanswerable, because the job id
934+
outlives the request that created it.
935+
936+
`partially_succeeded` is terminal and carries a message, so it is raised
937+
rather than returned -- a caller asked for a table's contents to be
938+
replaced or appended to, and "some of it" is not an answer it can use.
939+
"""
940+
final = self._poll_job(job_id, timeout_s=_LOAD_JOB_TIMEOUT_S)
941+
status = enum_value(final.status)
942+
if status != "succeeded":
943+
raise RuntimeError(
944+
final.error_message or f"load job {job_id} finished {status}"
945+
)
946+
payload = final.result.actual_instance if final.result is not None else None
947+
if not isinstance(payload, LoadManagedTableResponse):
948+
raise RuntimeError(
949+
f"load job {job_id} succeeded without a load result "
950+
f"(got {type(payload).__name__})"
951+
)
952+
return payload
953+
896954
def _wait_result_ready(
897955
self,
898956
result_id: str,

tests/test_client.py

Lines changed: 171 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,16 @@ def add_database_table(self, database_id, var_schema, request):
3737

3838

3939
class _FakeConnectionsApi:
40-
def __init__(self) -> None:
40+
def __init__(self, responses=None) -> None:
4141
self.load_calls: list[tuple[str, str, str]] = []
42+
self.requests: list = []
43+
self._responses = list(responses) if responses else None
4244

4345
def load_managed_table(self, connection_id, schema, table, request):
4446
self.load_calls.append((connection_id, schema, table))
47+
self.requests.append(request)
48+
if self._responses:
49+
return self._responses.pop(0)
4550
return SimpleNamespace(
4651
connection_id=connection_id,
4752
schema_name=schema,
@@ -544,3 +549,168 @@ def test_from_env_requires_an_api_key(monkeypatch: pytest.MonkeyPatch):
544549
monkeypatch.delenv("HOTDATA_API_KEY", raising=False)
545550
with pytest.raises(RuntimeError, match="HOTDATA_API_KEY"):
546551
HotdataClient.from_env()
552+
553+
554+
# --------------------------------------------------------------------------
555+
# A load is submitted as a job, not held open on one request
556+
# --------------------------------------------------------------------------
557+
558+
559+
def _load_response(rows=7):
560+
from hotdata.models.load_managed_table_response import LoadManagedTableResponse
561+
562+
return LoadManagedTableResponse(
563+
connection_id="conn_1", schema_name="public",
564+
table_name="orders", row_count=rows,
565+
arrow_schema_json="{}",
566+
)
567+
568+
569+
def test_a_load_asks_for_a_job_with_an_inline_window():
570+
"""The request itself is the fix. A load whose duration scales with the data
571+
must not depend on one HTTP request surviving minutes through every layer
572+
between here and the engine; when such a request dies the server discards the
573+
work and leaves the table locked against the retry."""
574+
from hotdata_framework.client import _LOAD_INLINE_WAIT_MS
575+
576+
client = HotdataClient("k", "ws", host="https://api.hotdata.dev")
577+
db = ManagedDatabase(id="db_1", description="mydb", default_connection_id="conn_1")
578+
connections = _FakeConnectionsApi()
579+
580+
with (
581+
patch.object(client, "_databases_api", return_value=_ForbiddenDatabasesApi()),
582+
patch.object(client, "connections", return_value=connections),
583+
):
584+
client.load_managed_table(db, "orders", schema="public", upload_id="up_1")
585+
586+
req = connections.requests[0]
587+
assert req.var_async is True, "load was not submitted as a job"
588+
assert req.async_after_ms == _LOAD_INLINE_WAIT_MS
589+
590+
591+
def test_a_load_that_finishes_inline_costs_no_polling():
592+
"""dlt's bookkeeping tables settle in under a second. Making those pay a
593+
submit-then-poll round trip would be a regression, which is what
594+
`async_after_ms` exists to prevent."""
595+
client = HotdataClient("k", "ws", host="https://api.hotdata.dev")
596+
db = ManagedDatabase(id="db_1", description="mydb", default_connection_id="conn_1")
597+
connections = _FakeConnectionsApi()
598+
599+
def _forbidden_poll(*a, **k):
600+
raise AssertionError("polled a load the server answered inline")
601+
602+
with (
603+
patch.object(client, "_databases_api", return_value=_ForbiddenDatabasesApi()),
604+
patch.object(client, "connections", return_value=connections),
605+
patch.object(client, "_poll_job", _forbidden_poll),
606+
):
607+
result = client.load_managed_table(db, "orders", schema="public", upload_id="up_1")
608+
609+
assert result.row_count == 3
610+
611+
612+
def test_a_load_the_server_defers_is_polled_to_completion():
613+
"""The 202 path: the result comes from durable job state rather than from a
614+
connection that had to stay alive to carry it."""
615+
from hotdata.models.submit_job_response import SubmitJobResponse
616+
617+
client = HotdataClient("k", "ws", host="https://api.hotdata.dev")
618+
db = ManagedDatabase(id="db_1", description="mydb", default_connection_id="conn_1")
619+
connections = _FakeConnectionsApi(responses=[
620+
SubmitJobResponse(id="jobs_1", status="running", status_url="/v1/jobs/jobs_1"),
621+
])
622+
final = SimpleNamespace(
623+
status="succeeded", error_message=None,
624+
result=SimpleNamespace(actual_instance=_load_response(rows=91)),
625+
)
626+
627+
with (
628+
patch.object(client, "_databases_api", return_value=_ForbiddenDatabasesApi()),
629+
patch.object(client, "connections", return_value=connections),
630+
patch.object(client, "_poll_job", return_value=final) as poll,
631+
):
632+
result = client.load_managed_table(db, "orders", schema="public", upload_id="up_1")
633+
634+
assert result.row_count == 91
635+
assert poll.call_args.args[0] == "jobs_1"
636+
637+
638+
def test_a_load_job_gets_its_own_budget_not_the_query_one():
639+
"""Reusing the 300s query budget is what put a five-minute ceiling on loads in
640+
the first place; a load is the one operation whose duration scales with the
641+
data."""
642+
from hotdata.models.submit_job_response import SubmitJobResponse
643+
from hotdata_framework.client import _LOAD_JOB_TIMEOUT_S
644+
645+
assert _LOAD_JOB_TIMEOUT_S > 300.0
646+
647+
client = HotdataClient("k", "ws", host="https://api.hotdata.dev")
648+
db = ManagedDatabase(id="db_1", description="mydb", default_connection_id="conn_1")
649+
connections = _FakeConnectionsApi(responses=[
650+
SubmitJobResponse(id="jobs_1", status="running", status_url="/v1/jobs/jobs_1"),
651+
])
652+
final = SimpleNamespace(
653+
status="succeeded", error_message=None,
654+
result=SimpleNamespace(actual_instance=_load_response()),
655+
)
656+
657+
with (
658+
patch.object(client, "_databases_api", return_value=_ForbiddenDatabasesApi()),
659+
patch.object(client, "connections", return_value=connections),
660+
patch.object(client, "_poll_job", return_value=final) as poll,
661+
):
662+
client.load_managed_table(db, "orders", schema="public", upload_id="up_1")
663+
664+
assert poll.call_args.kwargs["timeout_s"] == _LOAD_JOB_TIMEOUT_S
665+
666+
667+
def test_a_failed_load_job_raises_with_the_server_message():
668+
from hotdata.models.submit_job_response import SubmitJobResponse
669+
670+
client = HotdataClient("k", "ws", host="https://api.hotdata.dev")
671+
db = ManagedDatabase(id="db_1", description="mydb", default_connection_id="conn_1")
672+
connections = _FakeConnectionsApi(responses=[
673+
SubmitJobResponse(id="jobs_1", status="running", status_url="/v1/jobs/jobs_1"),
674+
])
675+
final = SimpleNamespace(status="failed", error_message="disk full", result=None)
676+
677+
with (
678+
patch.object(client, "_databases_api", return_value=_ForbiddenDatabasesApi()),
679+
patch.object(client, "connections", return_value=connections),
680+
patch.object(client, "_poll_job", return_value=final),
681+
):
682+
try:
683+
client.load_managed_table(db, "orders", schema="public", upload_id="up_1")
684+
except RuntimeError as e:
685+
assert "disk full" in str(e), e
686+
else:
687+
raise AssertionError("a failed load job did not raise")
688+
689+
690+
def test_a_partially_succeeded_load_job_is_not_treated_as_success():
691+
"""A caller asked for a table's contents to be replaced or appended to;
692+
"some of it" is not an answer it can use."""
693+
from hotdata.models.submit_job_response import SubmitJobResponse
694+
695+
client = HotdataClient("k", "ws", host="https://api.hotdata.dev")
696+
db = ManagedDatabase(id="db_1", description="mydb", default_connection_id="conn_1")
697+
connections = _FakeConnectionsApi(responses=[
698+
SubmitJobResponse(id="jobs_1", status="running", status_url="/v1/jobs/jobs_1"),
699+
])
700+
final = SimpleNamespace(
701+
status="partially_succeeded", error_message="3 rows rejected",
702+
result=SimpleNamespace(actual_instance=_load_response()),
703+
)
704+
705+
with (
706+
patch.object(client, "_databases_api", return_value=_ForbiddenDatabasesApi()),
707+
patch.object(client, "connections", return_value=connections),
708+
patch.object(client, "_poll_job", return_value=final),
709+
):
710+
try:
711+
client.load_managed_table(db, "orders", schema="public", upload_id="up_1")
712+
except RuntimeError as e:
713+
assert "3 rows rejected" in str(e), e
714+
else:
715+
raise AssertionError("partially_succeeded was treated as success")
716+

0 commit comments

Comments
 (0)