fix(cdk): emit interim heartbeat state for global-cursor substreams with empty parent state#1068
fix(cdk): emit interim heartbeat state for global-cursor substreams with empty parent state#1068devin-ai-integration[bot] wants to merge 4 commits into
Conversation
…ith empty parent state During a long parent walk with mostly-empty children, a global_substream_cursor substream without incremental_dependency emitted no RECORD and no STATE for the whole walk, tripping the Airbyte source heartbeat (86400s). ConcurrentPerPartitionCursor._emit_state_message suppressed the throttled checkpoint STATE whenever the parent state was empty, so the only keepalive signal was withheld exactly when it was needed. Remove the empty-parent guard so the existing 600s-throttled checkpoint STATE is emitted even when parent state is empty. The interim state carries the stable (unadvanced) global cursor, so it is inert on resume (no data loss, no re-read regression). The 600s throttle is unchanged. Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1783450708-heartbeat-guard-relax#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1783450708-heartbeat-guard-relaxPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
PyTest Results (Fast)4 134 tests +11 4 122 ✅ +11 8m 55s ⏱️ + 1m 19s Results for commit d66b623. ± Comparison against base commit 53785b6. This pull request removes 1 and adds 12 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
PyTest Results (Full)4 137 tests 4 125 ✅ 10m 40s ⏱️ Results for commit d66b623. ♻️ This comment has been updated with latest results. |
… without skipping records Co-Authored-By: bot_apk <apk@cognition.ai>
|
/prerelease
|
…e size bounded Add two tests for OC-12977 proving the guard removal does not balloon persisted connection state as the number of empty parents grows (Serhii's orchestrator-overload risk): state stays a single bounded global-cursor value with no per-partition map, byte-for-byte identical vs the pre-fix baseline, and the 600s throttle keeps emission count flat regardless of parent count. Co-Authored-By: bot_apk <apk@cognition.ai>
Adds four hardening checks around the empty-parent guard removal: - no-op test for the non-empty-parent-state (incremental_dependency) shape: records + STATE byte-identical guard-removed vs guard-restored, bounding the shared-CDK change's blast radius to the empty-parent case; - data-last fixture: many empty parents precede the single data-bearing partition, so resume is exercised from checkpoints emitted during the silent stretch before any record exists; - determinism: pin default_concurrency=1 and assert RECORD/STATE interleaving is identical across repeated runs (no concurrent-ordering flake); - assert resume meaningfully contributes the tail (records not already emitted before the checkpoint), so the test can't pass trivially. Co-Authored-By: bot_apk <apk@cognition.ai>
|
/prerelease
|
Summary
A
global_substream_cursorsubstream that walks a large parent with mostly-empty children can emit no RECORD and no STATE for the entire walk, tripping the Airbyte source heartbeat (heartbeat_timeout, threshold 86400s). This happened on a customer'ssource-stripeinitial sync (seeairbytehq/oncall#12977) for thecustomerschild streamsbank_accountsandcustomer_balance_transactions.Root cause:
ConcurrentPerPartitionCursor._emit_state_messagesuppressed the throttled checkpoint STATE whenever_parent_statewas empty:For a substream without
incremental_dependency,_parent_statestays empty for the whole walk, so every interim (per-close_partition) checkpoint is suppressed — the single mechanism that could keep the heartbeat alive is withheld exactly when it is needed. This PR removes that guard so the existing 600s-throttled checkpoint STATE flows even when parent state is empty.The interim STATE is inert on resume: on the global path,
close_partitionnever advances_global_cursor(onlyensure_at_least_one_state_emitteddoes, at the end of the sync —concurrent_partition_cursor.py:281), so an interim checkpoint carries the stable/unadvanced cursor. Resume from it re-reads rather than skipping → no data loss, no re-read regression. The 600s throttle is unchanged. The guard only ever applied to the global-cursor path (_use_global_cursor and ...), so per-partition state behavior is untouched.The fix
Diagnosis
Emission of interim STATE is driven entirely by
close_partition(which calls_emit_state_message(throttle=True)). Two distinct failure modes were investigated for the two failing streams:bank_accounts(SubstreamPartitionRouter,global_substream_cursor: true, noincremental_dependency):close_partitionfires repeatedly during the walk, but_parent_stateis empty → every throttled checkpoint hit the guard → 24h silence. This PR fixes it directly.customer_balance_transactions(incremental_dependency: true):SubstreamPartitionRouter.get_stream_state()returns{parent_name: parent.cursor.state}— a dict that always contains the parent key, hence truthy — so the guard was already bypassed for this stream. It is therefore not the cause of its silence. Its production symptom (0 records / 0 STATE for 24h) is a separate root cause: a stall in the parent-generation / parent-read path. Because emission is gated onclose_partition, if the parent read stalls, no partition closes and no state-emission change (including this one) can keep it alive. This aligns with the earlier concurrent-source RCA (synchronous parent reads holding partition-generator slots + unboundedqueue.get()) and is out of scope for this minimal fix — flagged for a follow-up.Fixture validation
A large-parent fixture (3000 parents over 30 slow pages, empty children) exercises the real cursor. The 600s throttle is compressed to 1s purely to simulate the many 600s windows that elapse over a 24h walk (production throttle unchanged). "STATE messages (output)" is the number that reached stdout:
incremental_dependencybank_accountsbank_accountscustomer_balance_transactionscustomer_balance_transactionscustomer_balance_transactions+ 6s parent stallThe stall row demonstrates the separate
customer_balance_transactionsroot cause: during the injected 6s parent-read stall,close_partitiondoes not fire, so the max gap between STATE messages equals the stall duration — no state-emission change can bridge it.Testing
test_global_cursor_emits_interim_state_even_when_parent_state_empty(parametrized over empty and non-empty parent state): the throttled checkpoint STATE is emitted in both cases for a global-cursor stream.test_incremental_parent_state_no_incremental_dependencyexpectation from 1 → 2 state messages for the resume case: the previously-suppressed interim keepalive is now emitted; the final state is unchanged (interim carries the same inert global cursor).test_global_cursor_substream_resume_after_interruption_skips_no_records): interrupt the walk at every interim checkpoint, resume from each, and assertbefore ∪ after == full expected set(deduped) so both skips and extras are caught. Non-vacuous: restoring the guard emits only 1 STATE and the test fails.test_guard_removal_is_a_noop_for_non_empty_parent_state): for a parent withincremental_dependency: true, the emitted records and the full STATE sequence are byte-for-byte identical guard-removed vs. guard-restored — bounding this shared-CDK change's blast radius to the empty-parent case.test_global_cursor_substream_resume_with_data_bearing_partition_last_skips_no_records): mirrors the real failure — many empty parents precede the single data-bearing partition, so resume is exercised from checkpoints emitted during the silent stretch before any record exists.default_concurrency=1and assert the RECORD/STATE interleaving is identical across repeated runs, so reconstructing "records committed before the kill" can't become a concurrent-ordering flake.statesmap accumulates and the 600s throttle holds emission to a small constant count regardless of parent count.unit_tests/sources/declarative/incremental/test_concurrent_perpartitioncursor.py: 34 passed.ruff format,ruff check,mypyclean.Context:
airbytehq/oncall#12977Link to Devin session: https://app.devin.ai/sessions/36bd90d019394ef59de523d61c4abc46