Chunked + concurrent get_range for long ranges (#33)#37
Conversation
The Historical guide showed `foreach_record(client, DBN.TradeMsg; ...)`, a method that does not exist — the call raises a MethodError. The record type is inferred from `schema` (overridable via the `record_type` kwarg). Corrected the example and the surrounding prose. Fixes #32 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Long-range get_range queries spend well over 100s in server-side assembly before the first byte streams, so the old 100s default killed every multi-year pull. Read timeouts are deterministic for a given query shape, so they are no longer retried; HTTP.jl TimeoutError is mapped to a new BentoTimeoutError whose message names the remedy. Fixes #31 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Historical responses can legitimately interleave gateway ErrorMsg, SystemMsg, and SymbolMappingMsg records with the data; the typed decode path threw on the first one, forcing callers back to typed=false. The new _foreach_typed_tolerant loop keeps the zero-allocation typed hot path, @Warns ErrorMsg contents (they explain missing data), quietly skips other control records, and skips mismatched/unknown rtypes with one summary warning. Fixes #30 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each get_range request carries ~25-30s of fixed server-side assembly latency for continuous-symbol queries, so long ranges either blow the read timeout as one request or accumulate hours of latency fetched sequentially. The new chunk kwarg (a Dates.Period) splits the range into half-open chunks fetched concurrently under a semaphore; records are concatenated in time order, failed chunks are warned and recorded in the new DBNStore.failed_ranges field for targeted retry. Fixes #33 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7e770ae4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| nxt = min(t + chunk, stop) | ||
| push!(chunks, (t, nxt)) | ||
| t = nxt |
There was a problem hiding this comment.
A valid CompoundPeriod can pass the one-time positivity check on line 185 but later stop advancing as calendar arithmetic changes near a month boundary. For example, with start_dt = DateTime(2024, 1, 1) and chunk = Month(1) - Day(30), the initial check advances to January 2, but eventually DateTime(2024, 1, 30) + chunk equals January 30. This loop then appends zero-length chunks indefinitely until memory exhaustion. Check that nxt > t inside the loop and reject the period otherwise.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2f6c8b4 — the upfront positivity check is replaced by an in-loop guard that requires advancement at every boundary. Note the literal example can't reach this code as written: Month(1) - Day(30) is a Dates.CompoundPeriod, which is not a subtype of Dates.Period, so it's rejected at the kwarg signature. But the in-loop guard is the right defense anyway (it also catches Day(0)/negative periods with a clear message, and survives a future widening of the kwarg type). Tests added.
The guide only imports DatabentoAPI and Dates, so the record_type override examples referenced an undefined DBN name (review feedback). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
If the foreach_record callback itself throws HTTP.Exceptions.TimeoutError (e.g. it makes its own HTTP request), open_stream unconditionally mapped it to BentoTimeoutError, blaming the Databento stream and reporting the wrong timeout value (review feedback). Track the exact exception object that escapes the consumer and map only stream-originated timeouts; a mid-body stall of our own stream still maps correctly because HTTP.jl raises the TimeoutError from the HTTP.open frame, a different object than whatever escaped the consumer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Calendar arithmetic means a period can advance from start_dt yet fail to advance at a later boundary, which would append zero-length chunks forever (review feedback). The in-loop guard replaces the single upfront positivity check and also covers Day(0)/negative periods. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
Top of the stack: #34 → #35 → #36 → this. Merge in order; each PR auto-retargets as its base branch is deleted.
Each
get_rangerequest for continuous-symbol queries carries ~25–30s of fixed server-side assembly latency regardless of payload size, so a 15-year pull is either one timeout-doomed request or ~an hour of accumulated latency over sequential chunks. This adds the chunk+concurrency pattern that worked in the field, built intoget_range:chunk::Union{Nothing,Dates.Period} = nothing—nothing(default) is exactly today''s single-request behavior; e.g.chunk = Year(1)splits[start_dt, end_dt)into half-open chunks (which compose exactly with Databento''s[start, end)semantics — no seam dedup needed).concurrency = 8— chunks fetch under aBase.SemaphoreviaThreads.@spawn; each task writes only its own result slot, so assembly is lock-free and deterministic in time order.HTTPClientis already task-safe and its 429 retry-with-backoff absorbs rate-limit brushes.chunkhint onBentoTimeoutError) and recorded in the newDBNStore.failed_rangesfield (Vector{Tuple{DateTime,DateTime}}) for targeted re-fetch. The call throws only if every chunk failed. The return type staysDBNStore{T}in both modes; the 2-arg constructors are preserved, so existing construction is unaffected.partial/not_found/mappingsunioned — they legitimately differ per date-chunk under continuous symbology).chunkrequires an explicitend_dtand calendar endpoints (DateTime/Date/parseable string — unix-ns ints can''t be split without assuming a timezone), and is incompatible withlimit;size_hintis ignored in chunked mode.Deferred from #33: the
encoding = "csv"escape hatch — will note on the issue as a follow-up.Tests
Full offline suite passes (1872 tests, Julia 1.12.6); docs build clean. New coverage: chunk splitting/clamping with exact wire
start/endparams, time-ordered concatenation, partial failure →failed_ranges, total failure → throws, all validation errors, single-request back-compat (1 dispatcher call, 2-arg constructors), and a concurrency high-water-mark gate test.Fixes #33
🤖 Generated with Claude Code