fix(bamboohr): stop the field-history build running out of memory - #2495
Conversation
The employee field-level change log diffed two per-version maps and read both back after an ARRAY JOIN, so every key replicated the whole payload and a single block cost gigabytes. A build large enough to matter was stopped by the server memory tracker, failing the sync after extraction had already committed its records. Shape it as GROUP BY aggregation over a per-field timeline instead. Aggregation state spills to disk past max_bytes_before_external_group_by, which the previous form could not do at all — ARRAY JOIN and window sort state both stay resident. Peak memory now stays near flat as the snapshot grows rather than tracking its size. Output is unchanged, including the clear a field emits when a later version stops carrying it. No max_memory_usage on the model: a self-imposed cap converts a build the server could still afford into a hard failure. Closes #2491 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe fields history macro now reconstructs per-field timelines with spillable grouped aggregation and emits changes from adjacent values. The BambooHR model adds ClickHouse settings for block size, thread count, and external group-by spilling. ChangesFields history processing
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The change is localized and no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant RawData
participant EntityVersions
participant FieldTimelines
participant HistoryOutput
RawData->>EntityVersions: Provide entity version timestamps
RawData->>FieldTimelines: Provide entity and field value pairs
EntityVersions->>FieldTimelines: Map values across all versions
FieldTimelines->>HistoryOutput: Emit changes between adjacent values
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
) (#2506) The employee field-level change log diffed two per-version maps and read both back after an ARRAY JOIN, so every key replicated the whole payload and a single block cost gigabytes. A build large enough to matter was stopped by the server memory tracker, failing the sync after extraction had already committed its records. Shape it as GROUP BY aggregation over a per-field timeline instead. Aggregation state spills to disk past max_bytes_before_external_group_by, which the previous form could not do at all — ARRAY JOIN and window sort state both stay resident. Peak memory now stays near flat as the snapshot grows rather than tracking its size. Output is unchanged, including the clear a field emits when a later version stops carrying it. No max_memory_usage on the model: a self-imposed cap converts a build the server could still afford into a hard failure. Closes #2491 (cherry picked from commit 3fbbc02) Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Problem
bamboohr__employees_fields_historydiffs two per-versionMap(String, String)payloads and reads both back after anARRAY JOINover the union of their keys. ClickHouse must therefore keep both maps alive across every replicated row, so one block costs (rows x keys x payload). The build is stopped by the server memory tracker withCode: 241 ... While executing ArrayJoinTransform, failing the sync after extraction has already committed its records and leaving the target table stale.The BambooHR employee stream is the worst case for this by design: it collects every non-sensitive field the account defines, so the key count per row is large on purpose.
Fix
Reshape
fields_history_rawas GROUP BY aggregation over a per-field timeline: exploderaw_datato(key, value)rows before any wide payload exists, group each (entity, field) into its value-per-version timeline, and derive the transitions with array functions.The point is not just fewer bytes — it is which operator holds the state. Aggregation state spills to disk past
max_bytes_before_external_group_by;ARRAY JOINand window-sort state do not spill at all, which is why the spill knobs were inert on the previous shape (and, per #1921, on the one before that).max_memory_usageis deliberately not set on the model. Testing it showed a self-imposed cap converting a build the server could still afford into a hard failure — the same trap as #1921.Verification
Local ClickHouse 25.7.5, synthetic fixtures only. Output byte-identical to the previous implementation on:
cityHash64checksumPeak memory no longer tracks snapshot size: 4x the data costs ~12% more memory. Spill to disk was confirmed active via
ExternalAggregationWritePart.dbt parseanddbt compileclean.Known limit
This raises the ceiling but does not remove it. The model is
materialized='table'over an append-only snapshot, so every sync recomputes the entire history — millions of exploded rows to emit a few hundred thousand. At a snapshot roughly an order of magnitude larger than the sizes above, the build becomes impractically slow even though it no longer exhausts memory.Computed transitions are immutable, so the durable answer is incremental materialization behind a watermark. That needs a design decision about the watermark and is left as follow-up rather than bundled here.
Scope
fields_history_rawhas one caller. The named-fieldfields_historyused by the other nine connectors has noARRAY JOINand carries only scalar columns; it is untouched and unaffected.Closes #2491
Refs #1804, #1921
Summary by CodeRabbit
Bug Fixes
Performance
Documentation