Skip to content

Commit d54aa34

Browse files
devin-ai-integration[bot]bot_apk
andcommitted
test(concurrent-cursor): verify empty-parent guard removal keeps state 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>
1 parent cf52551 commit d54aa34

1 file changed

Lines changed: 215 additions & 0 deletions

File tree

unit_tests/sources/declarative/incremental/test_concurrent_perpartitioncursor.py

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Copyright (c) 2024 Airbyte, Inc., all rights reserved.
22
import copy
3+
import re
4+
from contextlib import nullcontext
35
from copy import deepcopy
46
from datetime import datetime, timedelta
57
from typing import Any, List, Mapping, MutableMapping, Optional, Union
@@ -1273,6 +1275,219 @@ def test_global_cursor_substream_resume_after_interruption_skips_no_records():
12731275
)
12741276

12751277

1278+
# OC-12977 / Serhii's orchestrator-overload risk: removing the empty-parent guard makes a
1279+
# global_substream_cursor substream emit interim checkpoint STATE during a long empty-parent
1280+
# walk. The constants + helpers below build that substream (the bank_accounts shape) walking an
1281+
# arbitrary number of empty parents (comments whose votes are all empty) so we can measure the
1282+
# persisted state size as the empty-parent count grows.
1283+
MANY_EMPTY_PARENTS_UPDATED_AT = "2024-06-01T00:00:00Z" # >= START_DATE so no parent is filtered out
1284+
1285+
1286+
def _emit_state_message_with_guard(self, throttle: bool = True) -> None:
1287+
"""Pre-fix ``_emit_state_message`` with the empty-parent guard restored.
1288+
1289+
Used to reproduce the pre-fix baseline so we can compare persisted state size before vs.
1290+
after the guard removal on the exact same fixture.
1291+
"""
1292+
if throttle:
1293+
current_time = self._throttle_state_message()
1294+
if current_time is None:
1295+
return
1296+
self._last_emission_time = current_time
1297+
# The guard that PR #1068 removes:
1298+
if self._use_global_cursor and not self._parent_state:
1299+
return
1300+
1301+
self._connector_state_manager.update_state_for_stream(
1302+
self._stream_name,
1303+
self._stream_namespace,
1304+
self.state,
1305+
)
1306+
state_message = self._connector_state_manager.create_state_message(
1307+
self._stream_name, self._stream_namespace
1308+
)
1309+
self._message_repository.emit_message(state_message)
1310+
1311+
1312+
def _run_global_cursor_over_empty_parents(
1313+
num_empty_parents, num_non_empty_parents=0, reintroduce_guard=False
1314+
):
1315+
"""Run the global-cursor substream over ``num_empty_parents`` empty parents.
1316+
1317+
One post fans out to ``num_non_empty_parents + num_empty_parents`` comments (the parents).
1318+
The first ``num_non_empty_parents`` comments each return a single vote (advancing the global
1319+
cursor and populating the in-memory per-partition map), and the remaining ``num_empty_parents``
1320+
comments return empty votes. The only thing that could grow with the empty-parent count is the
1321+
persisted state, which is exactly what we measure. When ``reintroduce_guard`` is True the
1322+
pre-fix empty-parent guard is patched back in to produce the baseline for comparison.
1323+
"""
1324+
posts_response = {"posts": [{"id": 1, "updated_at": MANY_EMPTY_PARENTS_UPDATED_AT}]}
1325+
non_empty_ids = [1000 + i for i in range(num_non_empty_parents)]
1326+
empty_ids = [1000 + num_non_empty_parents + i for i in range(num_empty_parents)]
1327+
comments_response = {
1328+
"comments": [
1329+
{"id": cid, "post_id": 1, "updated_at": MANY_EMPTY_PARENTS_UPDATED_AT}
1330+
for cid in non_empty_ids + empty_ids
1331+
]
1332+
}
1333+
1334+
guard_ctx = (
1335+
patch.object(
1336+
ConcurrentPerPartitionCursor,
1337+
"_emit_state_message",
1338+
_emit_state_message_with_guard,
1339+
)
1340+
if reintroduce_guard
1341+
else nullcontext()
1342+
)
1343+
with guard_ctx, requests_mock.Mocker() as m:
1344+
# Every empty parent's votes call (one per comment) answers empty by default.
1345+
m.get(re.compile(r"/votes"), json={"votes": []})
1346+
# The non-empty parents each return a single vote (more specific match wins).
1347+
for idx, cid in enumerate(non_empty_ids):
1348+
m.get(
1349+
f"https://api.example.com/community/posts/1/comments/{cid}/votes?per_page=100&start_time={START_DATE}",
1350+
json={
1351+
"votes": [
1352+
{
1353+
"id": 900000 + idx,
1354+
"comment_id": cid,
1355+
"created_at": MANY_EMPTY_PARENTS_UPDATED_AT,
1356+
}
1357+
]
1358+
},
1359+
)
1360+
m.get(
1361+
f"https://api.example.com/community/posts?per_page=100&start_time={START_DATE}",
1362+
json=posts_response,
1363+
)
1364+
m.get(
1365+
"https://api.example.com/community/posts/1/comments?per_page=100",
1366+
json=comments_response,
1367+
)
1368+
return _run_read(
1369+
SUBSTREAM_MANIFEST_WITH_GLOBAL_CURSOR_AND_NO_DEPENDENCY,
1370+
CONFIG,
1371+
STREAM_NAME,
1372+
None,
1373+
)
1374+
1375+
1376+
def _persisted_state_size(state_message) -> int:
1377+
return len(orjson.dumps(state_message.state.stream.stream_state.__dict__))
1378+
1379+
1380+
def test_removing_empty_parent_guard_keeps_state_size_flat_as_empty_parents_grow():
1381+
"""OC-12977: prove removing the empty-parent guard does not balloon persisted state.
1382+
1383+
Serhii's flagged risk is that emitting interim checkpoint STATE for a ``global_substream_cursor``
1384+
substream with empty parents could grow the persisted connection state per empty parent and
1385+
overload the orchestrator. This test walks the same substream over a handful vs. thousands of
1386+
empty parents and asserts:
1387+
1388+
1. the persisted state carries a single bounded global-cursor value with **no** per-partition
1389+
``states`` map -- so there is no per-parent growth vector at all;
1390+
2. the persisted state is byte-for-byte identical at low vs. high empty-parent counts;
1391+
3. the persisted state is byte-for-byte identical to the pre-fix (guard-restored) baseline;
1392+
4. the 600s throttle keeps the emitted-state *count* a small constant, independent of the
1393+
parent count (we re-emit the same small state at most ~once/600s, not once per parent); and
1394+
5. the in-memory per-partition retention cap is unchanged (it bounds memory, not state).
1395+
"""
1396+
low, high = 5, 3000
1397+
1398+
low_output = _run_global_cursor_over_empty_parents(low)
1399+
high_output = _run_global_cursor_over_empty_parents(high)
1400+
1401+
# Pure empty-parent walk: no records emitted for either count.
1402+
assert len(low_output.records) == 0
1403+
assert len(high_output.records) == 0
1404+
1405+
low_final = low_output.state_messages[-1].state.stream.stream_state.__dict__
1406+
high_final = high_output.state_messages[-1].state.stream.stream_state.__dict__
1407+
1408+
# (1) Global mode never serializes a per-partition state map -> no per-parent growth vector.
1409+
assert "states" not in low_final
1410+
assert "states" not in high_final
1411+
1412+
# (2) Persisted state is byte-for-byte identical at low vs. high empty-parent counts.
1413+
assert orjson.dumps(low_final) == orjson.dumps(high_final)
1414+
low_size = len(orjson.dumps(low_final))
1415+
high_size = len(orjson.dumps(high_final))
1416+
assert low_size == high_size, (
1417+
f"Persisted state grew with empty-parent count: {low_size}B at {low} parents "
1418+
f"vs {high_size}B at {high} parents."
1419+
)
1420+
# Bounded: a single small global-cursor value, not kilobytes-per-parent.
1421+
assert high_size < 500, f"Persisted state unexpectedly large: {high_size}B"
1422+
# Every emitted state message (interim + final) is the same bounded size.
1423+
assert max(_persisted_state_size(m) for m in high_output.state_messages) < 500
1424+
1425+
# (3) Unchanged vs. the pre-fix (guard-restored) baseline on the same fixture.
1426+
baseline_output = _run_global_cursor_over_empty_parents(high, reintroduce_guard=True)
1427+
baseline_final = baseline_output.state_messages[-1].state.stream.stream_state.__dict__
1428+
assert orjson.dumps(high_final) == orjson.dumps(baseline_final), (
1429+
"Persisted final state differs from the pre-fix baseline; guard removal must not "
1430+
f"change state content. after={high_final} baseline={baseline_final}"
1431+
)
1432+
1433+
# (4) The 600s throttle keeps the emitted-state count a small constant, not per-parent.
1434+
assert len(high_output.state_messages) <= 3, (
1435+
"Expected the 600s throttle to bound state emissions to a small constant, got "
1436+
f"{len(high_output.state_messages)} state messages for {high} empty parents."
1437+
)
1438+
assert len(high_output.state_messages) == len(low_output.state_messages), (
1439+
"State-emission count scaled with empty-parent count: "
1440+
f"{len(low_output.state_messages)} at {low} vs {len(high_output.state_messages)} at {high}."
1441+
)
1442+
1443+
# (5) The in-memory per-partition retention cap is unchanged (bounds memory, not state).
1444+
assert ConcurrentPerPartitionCursor.DEFAULT_MAX_PARTITIONS_NUMBER == 25_000
1445+
assert ConcurrentPerPartitionCursor.SWITCH_TO_GLOBAL_LIMIT == 10_000
1446+
1447+
1448+
def test_persisted_state_has_no_per_partition_entries_even_when_cursor_advances():
1449+
"""The per-partition state map must not leak into persisted state, even with real records.
1450+
1451+
A skeptic might argue the flat state size above is only because a fully-empty walk produces
1452+
no cursor value. This test mixes a few non-empty parents (which advance the global cursor and
1453+
populate the in-memory per-partition map) among many empty parents, then proves the persisted
1454+
state still carries **no** ``states`` per-partition list and stays byte-for-byte identical as
1455+
the empty-parent count grows from a handful to thousands. This is the core guarantee behind the
1456+
orchestrator-overload risk: a ``global_substream_cursor`` stream serializes a single global
1457+
cursor value, never one entry per parent.
1458+
"""
1459+
low_output = _run_global_cursor_over_empty_parents(5, num_non_empty_parents=3)
1460+
high_output = _run_global_cursor_over_empty_parents(3000, num_non_empty_parents=3)
1461+
1462+
# The non-empty parents produced records, so the global cursor actually advanced.
1463+
assert len(low_output.records) == 3
1464+
assert len(high_output.records) == 3
1465+
1466+
low_final = low_output.state_messages[-1].state.stream.stream_state.__dict__
1467+
high_final = high_output.state_messages[-1].state.stream.stream_state.__dict__
1468+
1469+
# A real global cursor value is present now...
1470+
assert "state" in high_final
1471+
# ...but there is still no per-partition state map, regardless of the empty-parent count.
1472+
assert "states" not in low_final
1473+
assert "states" not in high_final
1474+
1475+
# The persisted state is identical at 5 vs. 3000 empty parents apart from ``lookback_window``,
1476+
# which is a small integer derived from the sync's wall-clock duration (not the parent count).
1477+
# Everything else -- the single global cursor value and the empty parent_state -- is unchanged.
1478+
def _without_lookback(state):
1479+
return {k: v for k, v in state.items() if k != "lookback_window"}
1480+
1481+
assert _without_lookback(low_final) == _without_lookback(high_final), (
1482+
f"Persisted state (excluding lookback_window) changed with empty-parent count: "
1483+
f"{low_final} vs {high_final}"
1484+
)
1485+
# And the total serialized size stays flat and bounded regardless of the empty-parent count
1486+
# (the only possible variation is a digit or two in the wall-clock-derived lookback_window).
1487+
assert abs(len(orjson.dumps(low_final)) - len(orjson.dumps(high_final))) <= 3
1488+
assert len(orjson.dumps(high_final)) < 500
1489+
1490+
12761491
def run_incremental_parent_state_test(
12771492
manifest,
12781493
mock_requests,

0 commit comments

Comments
 (0)