Skip to content

Commit 68bc46e

Browse files
committed
refactor(fork): page the re-home lookups and reuse the plan's identity rows
Self-review of the folder-transit change surfaced two scaling problems in the re-home pass, both of which grow with the size of the fork edge rather than the size of the sync: - The resource lookups built `IN (...)` lists straight from the edge's mapping rows, so a large fork could hand Postgres a list approaching the bind-parameter ceiling and a pathological query plan. Page them at 500, matching the paging the rest of the fork copy already uses. - The pass re-read the whole edge mapping via `getEdgeMappingRows`, which the promote plan had already loaded in the same transaction — a second full load of identical rows. Expose them on `ForkPromotePlan` and pass them in, which also drops a mock from the re-home tests. Also tally moved rows from `returning()` rather than the planned batch size, so the log line reports what the `folder_id IS NULL` guard actually wrote instead of what was attempted.
1 parent e317d2e commit 68bc46e

4 files changed

Lines changed: 174 additions & 153 deletions

File tree

apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { ForkEdge } from '@/ee/workspace-forking/lib/lineage/lineage'
88
import { detectForkCascadeReferences } from '@/ee/workspace-forking/lib/mapping/cascade'
99
import {
1010
buildForkResolver,
11+
type ForkMappingRow,
1112
getEdgeMappingRows,
1213
resourceTypeToForkKind,
1314
} from '@/ee/workspace-forking/lib/mapping/mapping-store'
@@ -89,6 +90,12 @@ export interface ForkPromotePlan {
8990
willUpdate: number
9091
willCreate: number
9192
willArchive: number
93+
/**
94+
* The edge's persisted identity rows, already read to build {@link ForkPromotePlan.resolver}.
95+
* Exposed so later stages of the same transaction reuse them instead of re-reading the whole
96+
* edge mapping, which for a large fork is a second full load of the same rows.
97+
*/
98+
mappingRows: ForkMappingRow[]
9299
}
93100

94101
/**
@@ -551,5 +558,6 @@ export async function computeForkPromotePlan(params: {
551558
willUpdate,
552559
willCreate,
553560
willArchive: archivedTargetIds.length,
561+
mappingRows,
554562
}
555563
}

apps/sim/ee/workspace-forking/lib/promote/promote.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ export async function promoteFork(params: PromoteForkParams): Promise<PromoteFor
567567
// no-op and never overrides a placement chosen in the target.
568568
const rehomeResult = await rehomeFlattenedForkResources({
569569
tx,
570-
edge,
570+
mappingRows: plan.mappingRows,
571571
sourceWorkspaceId,
572572
targetWorkspaceId,
573573
direction,

apps/sim/ee/workspace-forking/lib/promote/rehome-mapped.test.ts

Lines changed: 40 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,7 @@
44
import { folder as folderTable, knowledgeBase, workspaceFiles } from '@sim/db/schema'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
66
import type { DbOrTx } from '@/lib/db/types'
7-
8-
const { mockGetEdgeMappingRows } = vi.hoisted(() => ({
9-
mockGetEdgeMappingRows: vi.fn(),
10-
}))
11-
12-
vi.mock('@/ee/workspace-forking/lib/mapping/mapping-store', () => ({
13-
getEdgeMappingRows: mockGetEdgeMappingRows,
14-
}))
15-
7+
import type { ForkMappingRow } from '@/ee/workspace-forking/lib/mapping/mapping-store'
168
import { rehomeFlattenedForkResources } from '@/ee/workspace-forking/lib/promote/rehome-mapped'
179

1810
interface UpdateCall {
@@ -55,21 +47,38 @@ function makeTx(rows: {
5547
set: (values: Record<string, unknown>) => ({
5648
where: () => {
5749
updates.push({ table, values })
58-
return Promise.resolve()
50+
// The real update reports the rows it actually moved; echo one id back so the
51+
// caller's tally reflects a genuine write rather than the planned batch size.
52+
return { returning: () => Promise.resolve([{ id: 'moved' }]) }
5953
},
6054
}),
6155
}),
6256
}
6357
return { tx: tx as unknown as DbOrTx, updates, insertedFolders }
6458
}
6559

66-
const edge = {
67-
childWorkspaceId: 'child-ws',
68-
parentWorkspaceId: 'parent-ws',
69-
} as Parameters<typeof rehomeFlattenedForkResources>[0]['edge']
60+
const fileMapping: ForkMappingRow[] = [
61+
{
62+
id: 'map-1',
63+
childWorkspaceId: 'child-ws',
64+
resourceType: 'file',
65+
parentResourceId: 'workspace/parent-ws/a.png',
66+
childResourceId: 'workspace/child-ws/a.png',
67+
},
68+
]
69+
70+
const kbMapping: ForkMappingRow[] = [
71+
{
72+
id: 'map-2',
73+
childWorkspaceId: 'child-ws',
74+
resourceType: 'knowledge_base',
75+
parentResourceId: 'kb-target',
76+
childResourceId: 'kb-source',
77+
},
78+
]
7079

7180
const baseParams = {
72-
edge,
81+
mappingRows: [] as ForkMappingRow[],
7382
sourceWorkspaceId: 'child-ws',
7483
targetWorkspaceId: 'parent-ws',
7584
direction: 'push' as const,
@@ -80,20 +89,10 @@ const baseParams = {
8089
describe('rehomeFlattenedForkResources', () => {
8190
beforeEach(() => {
8291
vi.clearAllMocks()
83-
mockGetEdgeMappingRows.mockResolvedValue([])
8492
})
8593

8694
it('mirrors the source folder and moves a root-flattened mapped file into it', async () => {
8795
// Push: the child is the source, so the mapping row's child side is the source key.
88-
mockGetEdgeMappingRows.mockResolvedValue([
89-
{
90-
id: 'map-1',
91-
childWorkspaceId: 'child-ws',
92-
resourceType: 'file',
93-
parentResourceId: 'workspace/parent-ws/a.png',
94-
childResourceId: 'workspace/child-ws/a.png',
95-
},
96-
])
9796
const { tx, updates, insertedFolders } = makeTx({
9897
// Both the target lookup (flattened row) and the source lookup read this table; the
9998
// rows carry the fields each phase needs.
@@ -113,7 +112,11 @@ describe('rehomeFlattenedForkResources', () => {
113112
],
114113
})
115114

116-
const result = await rehomeFlattenedForkResources({ ...baseParams, tx })
115+
const result = await rehomeFlattenedForkResources({
116+
...baseParams,
117+
mappingRows: fileMapping,
118+
tx,
119+
})
117120

118121
expect(insertedFolders).toHaveLength(1)
119122
expect(insertedFolders[0]).toMatchObject({
@@ -130,39 +133,25 @@ describe('rehomeFlattenedForkResources', () => {
130133
})
131134

132135
it('leaves a resource alone when the source itself sits at the root', async () => {
133-
mockGetEdgeMappingRows.mockResolvedValue([
134-
{
135-
id: 'map-1',
136-
childWorkspaceId: 'child-ws',
137-
resourceType: 'file',
138-
parentResourceId: 'workspace/parent-ws/a.png',
139-
childResourceId: 'workspace/child-ws/a.png',
140-
},
141-
])
142136
const { tx, updates, insertedFolders } = makeTx({
143137
files: [
144138
{ id: 'file-target', key: 'workspace/parent-ws/a.png', folderId: null },
145139
{ id: 'file-source', key: 'workspace/child-ws/a.png', folderId: null },
146140
],
147141
})
148142

149-
const result = await rehomeFlattenedForkResources({ ...baseParams, tx })
143+
const result = await rehomeFlattenedForkResources({
144+
...baseParams,
145+
mappingRows: fileMapping,
146+
tx,
147+
})
150148

151149
expect(insertedFolders).toHaveLength(0)
152150
expect(updates).toHaveLength(0)
153151
expect(result.rehomed.file).toBe(0)
154152
})
155153

156154
it('never touches a target already placed in a folder, so a deliberate move survives a re-sync', async () => {
157-
mockGetEdgeMappingRows.mockResolvedValue([
158-
{
159-
id: 'map-1',
160-
childWorkspaceId: 'child-ws',
161-
resourceType: 'knowledge_base',
162-
parentResourceId: 'kb-target',
163-
childResourceId: 'kb-source',
164-
},
165-
])
166155
// The target read filters on `folderId IS NULL`, so an already-placed row is simply absent.
167156
const { tx, updates } = makeTx({
168157
knowledgeBases: [],
@@ -178,7 +167,11 @@ describe('rehomeFlattenedForkResources', () => {
178167
],
179168
})
180169

181-
const result = await rehomeFlattenedForkResources({ ...baseParams, tx })
170+
const result = await rehomeFlattenedForkResources({
171+
...baseParams,
172+
mappingRows: kbMapping,
173+
tx,
174+
})
182175

183176
expect(updates).toHaveLength(0)
184177
expect(result.rehomed.knowledge_base).toBe(0)
@@ -195,15 +188,6 @@ describe('rehomeFlattenedForkResources', () => {
195188
})
196189

197190
it('orients pull the other way: the parent side is the source', async () => {
198-
mockGetEdgeMappingRows.mockResolvedValue([
199-
{
200-
id: 'map-1',
201-
childWorkspaceId: 'child-ws',
202-
resourceType: 'file',
203-
parentResourceId: 'workspace/parent-ws/a.png',
204-
childResourceId: 'workspace/child-ws/a.png',
205-
},
206-
])
207191
const { tx, updates } = makeTx({
208192
files: [
209193
// On a pull the CHILD is the target, so its key is the one that must still be flattened.
@@ -224,6 +208,7 @@ describe('rehomeFlattenedForkResources', () => {
224208

225209
const result = await rehomeFlattenedForkResources({
226210
...baseParams,
211+
mappingRows: fileMapping,
227212
tx,
228213
direction: 'pull',
229214
sourceWorkspaceId: 'parent-ws',

0 commit comments

Comments
 (0)