Skip to content

Commit ad878e7

Browse files
baozhoutaoclaude
andauthored
fix(objectql): archive 冷侧 keep prune 尊重 #4747 的 teardown abort 位 (#6397)
PR #5956 给 archiveObject() 的批循环补上了 abort 检查,循环之后那条腿 没跟上:批循环刚因为读到 aborted === true 而 break,紧接着仍会向正在 关闭的 cold datasource 发一次 keep 保留期的谓词 DELETE。teardown 落在 最后一批热删里时同样如此 —— 那时循环按短页正常退出,不会再读一次 abort 位。 冷侧 prune 是纯保留期回收,不受「归档成功才热删」的配对约束,推迟到 下一轮 sweep 不留任何不一致。 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We Co-authored-by: Claude <noreply@anthropic.com>
1 parent e195092 commit ad878e7

3 files changed

Lines changed: 138 additions & 2 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
Lifecycle Archiver 的冷侧 `keep` prune 现在也尊重 #4747 的 teardown abort 位。
6+
7+
PR #5956`archiveObject()` 的批循环补上了 abort 检查,但循环**之后**那条腿 —— `archive.keep` 保留期在归档库上的谓词 DELETE —— 没跟上:批循环刚因为读到 `aborted === true` 而 break,紧接着仍会向正在关闭的 cold datasource 发一次 `deleteMany`。teardown 落在最后一批的热删里时同样如此,那时循环是按短页正常退出的,连再读一次 abort 位的机会都没有。
8+
9+
冷侧 prune 是纯保留期回收,不像循环内的 `upsert``bulkDelete` 那样受「归档成功才热删」的配对约束,推迟到下一轮 sweep 不留任何不一致 —— 下一轮按同一个 `keep` 推出同一个 cutoff,清同一批行。

packages/objectql/src/lifecycle/lifecycle-service.test.ts

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1643,10 +1643,16 @@ describe('LifecycleService teardown (#4747)', () => {
16431643
const pageReads: number[] = [];
16441644
const copied: string[] = [];
16451645
const hotDeleted: Array<Array<string | number>> = [];
1646+
// [#5966] The cold-side `keep` prune is the loop's successor leg, so it is
1647+
// recorded the same way the loop's legs are — with the predicate it sent,
1648+
// not just a count, so "the prune ran" and "the prune ran on the right
1649+
// cutoff" are separable assertions.
1650+
const coldPruned: Array<Record<string, unknown> | undefined> = [];
16461651
return {
16471652
pageReads,
16481653
copied,
16491654
hotDeleted,
1655+
coldPruned,
16501656
remaining: () => remaining,
16511657
hot: {
16521658
name: 'default',
@@ -1672,7 +1678,10 @@ describe('LifecycleService teardown (#4747)', () => {
16721678
return row;
16731679
},
16741680
bulkDelete: async () => {},
1675-
deleteMany: async () => 0,
1681+
deleteMany: async (_object: string, query?: Record<string, unknown>) => {
1682+
coldPruned.push(query);
1683+
return 0;
1684+
},
16761685
},
16771686
};
16781687
}
@@ -1727,6 +1736,109 @@ describe('LifecycleService teardown (#4747)', () => {
17271736
expect(report.swept.find((e) => e.policy === 'archive')?.archived).toBe(1000);
17281737
});
17291738

1739+
/**
1740+
* [#5966] The leg AFTER the loop. #5755 stopped the batch loop; the cold-side
1741+
* `keep` prune sits past its exit and had no check of its own, so teardown
1742+
* landing anywhere inside `archiveObject` still ended with one predicate
1743+
* DELETE at the cold datasource.
1744+
*
1745+
* The distinction that makes this worth pinning separately: by the time the
1746+
* prune is reached, the abort bit has already been READ — either the loop
1747+
* broke on it, or a leg the loop issued raised it. Continuing is a decision
1748+
* the code makes with the answer in hand, not an await that merely straddled
1749+
* teardown. The three tests below cover the prune's two ends and both ways
1750+
* the loop can hand control to it.
1751+
*/
1752+
const KEEP_OBJ: LifecycleObjectLike = {
1753+
name: 'sys_audit_log',
1754+
lifecycle: {
1755+
class: 'audit',
1756+
retention: { maxAge: '90d' },
1757+
// The delta from ARCHIVED_OBJ is `keep`, and only `keep`: the prune leg
1758+
// does not run at all without it, which is why #5755's fixture omitted it.
1759+
archive: { after: '90d', to: 'archive', keep: '365d' },
1760+
} as any,
1761+
};
1762+
1763+
it('a `keep` prune nobody calls off runs, on the cutoff `keep` declares', async () => {
1764+
// The control: with the abort bit down the prune is ordinary work, and the
1765+
// guard must not cost it. 700 rows drain inside the budget, so the loop
1766+
// exits on its short page — not on abort — and the prune follows it.
1767+
const pair = archivePair(700);
1768+
const { engine } = captureEngine([KEEP_OBJ], {
1769+
driver: pair.hot,
1770+
datasources: { archive: pair.cold },
1771+
});
1772+
1773+
const report = await service(engine).sweep();
1774+
1775+
expect(pair.pageReads).toHaveLength(2);
1776+
expect(pair.copied).toHaveLength(700);
1777+
expect(pair.remaining()).toHaveLength(0);
1778+
// Exactly one prune, carrying the `keep` cutoff (not `after`'s).
1779+
expect(pair.coldPruned).toEqual([{ where: { created_at: { $lt: isoCutoff('365d') } } }]);
1780+
expect(report.swept.find((e) => e.policy === 'archive')?.archived).toBe(700);
1781+
});
1782+
1783+
it('stop() mid-archive stops the prune too, not just the batch loop', async () => {
1784+
// Teardown lands while batch 2 copies, so the loop breaks at its head after
1785+
// READING `aborted === true` — and the very next statement used to send a
1786+
// predicate DELETE to the cold store the host is closing.
1787+
let svc!: LifecycleService;
1788+
const pair = archivePair(10_500, (copied) => {
1789+
if (copied === 501) svc.stop(); // first row of batch 2
1790+
});
1791+
const { engine } = captureEngine([KEEP_OBJ], {
1792+
driver: pair.hot,
1793+
datasources: { archive: pair.cold },
1794+
});
1795+
svc = service(engine);
1796+
1797+
const report = await svc.sweep();
1798+
1799+
expect(svc.stopped).toBe(true);
1800+
expect(pair.pageReads).toHaveLength(2); // #5755's guard, still holding
1801+
expect(pair.copied).toHaveLength(1000);
1802+
// The leg this test exists for: nothing at all is sent to the cold store
1803+
// after the loop reads the bit.
1804+
expect(pair.coldPruned).toEqual([]);
1805+
// Deferral, not loss: the rows the prune would have taken are still cold,
1806+
// and the archiving that DID complete is still reported.
1807+
expect(pair.remaining()).toHaveLength(9500);
1808+
expect(report.swept.find((e) => e.policy === 'archive')?.archived).toBe(1000);
1809+
});
1810+
1811+
it('stop() after the loop has made its last check still calls the prune off', async () => {
1812+
// The other way in, and the one the per-batch check cannot see: the backlog
1813+
// drains inside the budget, so the loop exits on `rows.length <
1814+
// ARCHIVE_BATCH_SIZE` and never re-reads the abort bit. Teardown lands in
1815+
// the final hot delete — after the loop's last check, before the prune —
1816+
// which is precisely the window #5755 left open.
1817+
let svc!: LifecycleService;
1818+
const pair = archivePair(700);
1819+
const hotBulkDelete = pair.hot.bulkDelete;
1820+
pair.hot.bulkDelete = async (object: string, ids: Array<string | number>) => {
1821+
await hotBulkDelete(object, ids);
1822+
if (pair.remaining().length === 0) svc.stop(); // the last batch just landed
1823+
};
1824+
const { engine } = captureEngine([KEEP_OBJ], {
1825+
driver: pair.hot,
1826+
datasources: { archive: pair.cold },
1827+
});
1828+
svc = service(engine);
1829+
1830+
const report = await svc.sweep();
1831+
1832+
expect(svc.stopped).toBe(true);
1833+
// The archive itself completed — every row copied and hot-deleted in pairs.
1834+
expect(pair.pageReads).toHaveLength(2);
1835+
expect(pair.hotDeleted.map((ids) => ids.length)).toEqual([500, 200]);
1836+
expect(pair.hotDeleted.flat()).toEqual(pair.copied);
1837+
expect(report.swept.find((e) => e.policy === 'archive')?.archived).toBe(700);
1838+
// …and the prune, the one leg still owed, is left for the next sweep.
1839+
expect(pair.coldPruned).toEqual([]);
1840+
});
1841+
17301842
it('stop() then start() re-arms the service — teardown is not one-way', async () => {
17311843
const { engine } = captureEngine([]);
17321844
let audits = 0;

packages/objectql/src/lifecycle/lifecycle-service.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,22 @@ export class LifecycleService {
10721072
}
10731073

10741074
// Cold-side retention: `keep` bounds the archive itself.
1075-
if (archive.keep && typeof cold.deleteMany === 'function') {
1075+
//
1076+
// [#4747] Leg boundary, after the batch loop — the last leg `archiveObject`
1077+
// can issue, and the one the per-batch check above does not reach. The loop
1078+
// may have just broken BECAUSE it read `aborted === true`, so firing a
1079+
// predicate DELETE at the cold datasource here is not a race teardown lost:
1080+
// it is a write issued by code that had already been told the engine is
1081+
// going away. That is what separates this leg from a lone `await` sitting
1082+
// between two checkpoints (the pre-#5194 reap shape) — there, nothing had
1083+
// observed the bit, and carrying on was not a decision.
1084+
//
1085+
// Deferring costs nothing. The cold prune is pure retention reclaim, not
1086+
// half of a pair: unlike the loop's `upsert` → `bulkDelete`, which must
1087+
// finish so the Archiver never hot-deletes a row the cold store has not
1088+
// taken, nothing is left inconsistent by skipping it. The next sweep
1089+
// re-derives the same cutoff from the same `keep` and prunes the same rows.
1090+
if (!this.abort.aborted && archive.keep && typeof cold.deleteMany === 'function') {
10761091
const keepCutoff = new Date(this.now() - parseLifecycleDuration(archive.keep)).toISOString();
10771092
await cold.deleteMany(object, { where: { created_at: { $lt: keepCutoff } } });
10781093
}

0 commit comments

Comments
 (0)