catalog: simplify stitch_queue to single ticket, fix down migration
Remove unused active_ticket column and rename latest_ticket back to stitch_ticket. Use knex.batchInsert in the up migration. Fix the down migration to restore stitch_ticket on final_entities without a server-side default, matching the original schema exactly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Fredrik Adelöw <freben@spotify.com>
This commit is contained in:
@@ -2,4 +2,4 @@
|
||||
'@backstage/plugin-catalog-backend': patch
|
||||
---
|
||||
|
||||
Moved stitch queue concerns out of `refresh_state` and `final_entities` into a dedicated `stitch_queue` table with `entity_ref` as the primary key. The stitch queue uses a two-ticket model (`latest_ticket` and `active_ticket`) for optimistic concurrency control. When a stitch completes successfully and the ticket hasn't changed, the corresponding row is deleted from the queue. The migration handles existing data and is fully reversible.
|
||||
Moved stitch queue concerns out of `refresh_state` and `final_entities` into a dedicated `stitch_queue` table with `entity_ref` as the primary key. The `stitch_ticket` is used for optimistic concurrency control. When a stitch completes successfully and the ticket hasn't changed, the corresponding row is deleted from the queue. The migration handles existing data and is fully reversible.
|
||||
|
||||
@@ -34,13 +34,9 @@ exports.up = async function up(knex) {
|
||||
.notNullable()
|
||||
.comment('The entity ref that needs stitching');
|
||||
table
|
||||
.string('latest_ticket')
|
||||
.string('stitch_ticket')
|
||||
.notNullable()
|
||||
.comment('Changes with every new stitch request');
|
||||
table
|
||||
.string('active_ticket')
|
||||
.nullable()
|
||||
.comment('Set when a stitcher picks up this item for processing');
|
||||
table
|
||||
.dateTime('next_stitch_at')
|
||||
.notNullable()
|
||||
@@ -58,16 +54,15 @@ exports.up = async function up(knex) {
|
||||
.from('refresh_state')
|
||||
.whereNotNull('refresh_state.next_stitch_at');
|
||||
|
||||
for (let i = 0; i < pendingStitches.length; i += 1000) {
|
||||
const batch = pendingStitches.slice(i, i + 1000);
|
||||
await knex('stitch_queue').insert(
|
||||
batch.map(row => ({
|
||||
entity_ref: row.entity_ref,
|
||||
latest_ticket: row.next_stitch_ticket || '',
|
||||
next_stitch_at: row.next_stitch_at,
|
||||
})),
|
||||
);
|
||||
}
|
||||
await knex.batchInsert(
|
||||
'stitch_queue',
|
||||
pendingStitches.map(row => ({
|
||||
entity_ref: row.entity_ref,
|
||||
stitch_ticket: row.next_stitch_ticket || '',
|
||||
next_stitch_at: row.next_stitch_at,
|
||||
})),
|
||||
1000,
|
||||
);
|
||||
|
||||
// Step 3: Remove next_stitch_at and next_stitch_ticket columns from refresh_state
|
||||
if (isSQLite) {
|
||||
@@ -102,10 +97,17 @@ exports.down = async function down(knex) {
|
||||
await knex.schema.alterTable('final_entities', table => {
|
||||
table
|
||||
.text('stitch_ticket')
|
||||
.notNullable()
|
||||
.defaultTo('')
|
||||
.comment('Optimistic concurrency ticket for stitching');
|
||||
.nullable()
|
||||
.comment('Random value representing a unique stitch attempt ticket');
|
||||
});
|
||||
await knex('final_entities').update({ stitch_ticket: '' });
|
||||
if (isSQLite) {
|
||||
// SQLite doesn't support ALTER COLUMN, but the nullable column is fine
|
||||
} else {
|
||||
await knex.schema.alterTable('final_entities', table => {
|
||||
table.text('stitch_ticket').notNullable().alter();
|
||||
});
|
||||
}
|
||||
|
||||
// Step 2: Add back the columns to refresh_state
|
||||
await knex.schema.alterTable('refresh_state', table => {
|
||||
@@ -140,7 +142,7 @@ exports.down = async function down(knex) {
|
||||
.select({
|
||||
entityRef: 'stitch_queue.entity_ref',
|
||||
nextStitchAt: 'stitch_queue.next_stitch_at',
|
||||
latestTicket: 'stitch_queue.latest_ticket',
|
||||
latestTicket: 'stitch_queue.stitch_ticket',
|
||||
})
|
||||
.from('stitch_queue');
|
||||
|
||||
@@ -160,7 +162,7 @@ exports.down = async function down(knex) {
|
||||
.from('stitch_queue')
|
||||
.whereRaw('stitch_queue.entity_ref = refresh_state.entity_ref'),
|
||||
next_stitch_ticket: knex
|
||||
.select('stitch_queue.latest_ticket')
|
||||
.select('stitch_queue.stitch_ticket')
|
||||
.from('stitch_queue')
|
||||
.whereRaw('stitch_queue.entity_ref = refresh_state.entity_ref'),
|
||||
})
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
> Do not edit this file. It is a report generated by `yarn build:api-reports`
|
||||
|
||||
> [!WARNING]
|
||||
> Failed to migrate down from '20260215000000_move_stitch_queue.js'
|
||||
> Failed to migrate down from '20241003170511_alter_target_in_locations.js'
|
||||
|
||||
## Sequences
|
||||
|
||||
@@ -136,10 +136,9 @@
|
||||
|
||||
| Column | Type | Nullable | Max Length | Default |
|
||||
| ---------------- | -------------------------- | -------- | ---------- | ------- |
|
||||
| `active_ticket` | `character varying` | true | 255 | - |
|
||||
| `entity_ref` | `character varying` | false | 255 | - |
|
||||
| `latest_ticket` | `character varying` | false | 255 | - |
|
||||
| `next_stitch_at` | `timestamp with time zone` | false | - | - |
|
||||
| `stitch_ticket` | `character varying` | false | 255 | - |
|
||||
|
||||
### Indices
|
||||
|
||||
|
||||
+3
-3
@@ -35,17 +35,17 @@ describe('getDeferredStitchableEntities', () => {
|
||||
await knex<DbStitchQueueRow>('stitch_queue').insert([
|
||||
{
|
||||
entity_ref: 'k:ns/future_stitch_time',
|
||||
latest_ticket: 't1',
|
||||
stitch_ticket: 't1',
|
||||
next_stitch_at: '2037-01-01T00:00:00.000',
|
||||
},
|
||||
{
|
||||
entity_ref: 'k:ns/past_stitch_time',
|
||||
latest_ticket: 't3',
|
||||
stitch_ticket: 't3',
|
||||
next_stitch_at: '1971-01-01T00:00:00.000',
|
||||
},
|
||||
{
|
||||
entity_ref: 'k:ns/past_stitch_time_again',
|
||||
latest_ticket: 't4',
|
||||
stitch_ticket: 't4',
|
||||
next_stitch_at: '1972-01-01T00:00:00.000',
|
||||
},
|
||||
]);
|
||||
|
||||
+3
-4
@@ -31,7 +31,7 @@ import { DbStitchQueueRow } from '../../tables';
|
||||
*
|
||||
* This assumes that the stitching strategy is set to deferred.
|
||||
*
|
||||
* They are expected to already have the latest_ticket set (by
|
||||
* They are expected to already have the stitch_ticket set (by
|
||||
* markForStitching) so that their tickets can be returned with each item.
|
||||
*
|
||||
* All returned items have their next_stitch_at updated to be moved forward by
|
||||
@@ -55,7 +55,7 @@ export async function getDeferredStitchableEntities(options: {
|
||||
let itemsQuery = knex<DbStitchQueueRow>('stitch_queue').select(
|
||||
'entity_ref',
|
||||
'next_stitch_at',
|
||||
'latest_ticket',
|
||||
'stitch_ticket',
|
||||
);
|
||||
|
||||
// This avoids duplication of work because of race conditions and is
|
||||
@@ -81,12 +81,11 @@ export async function getDeferredStitchableEntities(options: {
|
||||
)
|
||||
.update({
|
||||
next_stitch_at: nowPlus(knex, stitchTimeout),
|
||||
active_ticket: knex.ref('latest_ticket'),
|
||||
});
|
||||
|
||||
return items.map(i => ({
|
||||
entityRef: i.entity_ref,
|
||||
stitchTicket: i.latest_ticket,
|
||||
stitchTicket: i.stitch_ticket,
|
||||
stitchRequestedAt: timestampToDateTime(i.next_stitch_at),
|
||||
}));
|
||||
}
|
||||
|
||||
+3
-3
@@ -34,7 +34,7 @@ describe('markDeferredStitchCompleted', () => {
|
||||
await knex<DbStitchQueueRow>('stitch_queue').insert([
|
||||
{
|
||||
entity_ref: 'k:ns/n',
|
||||
latest_ticket: 'the-ticket',
|
||||
stitch_ticket: 'the-ticket',
|
||||
next_stitch_at: '1971-01-01T00:00:00.000',
|
||||
},
|
||||
]);
|
||||
@@ -43,7 +43,7 @@ describe('markDeferredStitchCompleted', () => {
|
||||
return knex<DbStitchQueueRow>('stitch_queue').select(
|
||||
'entity_ref',
|
||||
'next_stitch_at',
|
||||
'latest_ticket',
|
||||
'stitch_ticket',
|
||||
);
|
||||
}
|
||||
|
||||
@@ -57,7 +57,7 @@ describe('markDeferredStitchCompleted', () => {
|
||||
{
|
||||
entity_ref: 'k:ns/n',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: 'the-ticket',
|
||||
stitch_ticket: 'the-ticket',
|
||||
},
|
||||
]);
|
||||
|
||||
|
||||
+1
-1
@@ -38,6 +38,6 @@ export async function markDeferredStitchCompleted(option: {
|
||||
|
||||
await knex<DbStitchQueueRow>('stitch_queue')
|
||||
.where('entity_ref', '=', entityRef)
|
||||
.andWhere('latest_ticket', '=', stitchTicket)
|
||||
.andWhere('stitch_ticket', '=', stitchTicket)
|
||||
.delete();
|
||||
}
|
||||
|
||||
@@ -76,14 +76,14 @@ describe('markForStitching', () => {
|
||||
await knex<DbStitchQueueRow>('stitch_queue').insert([
|
||||
{
|
||||
entity_ref: 'k:ns/four',
|
||||
latest_ticket: 'old',
|
||||
stitch_ticket: 'old',
|
||||
next_stitch_at: '1971-01-01T00:00:00.000',
|
||||
},
|
||||
]);
|
||||
|
||||
async function result() {
|
||||
return knex<DbStitchQueueRow>('stitch_queue')
|
||||
.select('entity_ref', 'next_stitch_at', 'latest_ticket')
|
||||
.select('entity_ref', 'next_stitch_at', 'stitch_ticket')
|
||||
.orderBy('entity_ref', 'asc');
|
||||
}
|
||||
|
||||
@@ -93,7 +93,7 @@ describe('markForStitching', () => {
|
||||
{
|
||||
entity_ref: 'k:ns/four',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: 'old',
|
||||
stitch_ticket: 'old',
|
||||
},
|
||||
]);
|
||||
|
||||
@@ -111,7 +111,7 @@ describe('markForStitching', () => {
|
||||
{
|
||||
entity_ref: 'k:ns/four',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: 'old',
|
||||
stitch_ticket: 'old',
|
||||
},
|
||||
]);
|
||||
|
||||
@@ -129,12 +129,12 @@ describe('markForStitching', () => {
|
||||
{
|
||||
entity_ref: 'k:ns/four',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: 'old',
|
||||
stitch_ticket: 'old',
|
||||
},
|
||||
{
|
||||
entity_ref: 'k:ns/one',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: expect.anything(),
|
||||
stitch_ticket: expect.anything(),
|
||||
},
|
||||
]);
|
||||
|
||||
@@ -152,17 +152,17 @@ describe('markForStitching', () => {
|
||||
{
|
||||
entity_ref: 'k:ns/four',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: 'old',
|
||||
stitch_ticket: 'old',
|
||||
},
|
||||
{
|
||||
entity_ref: 'k:ns/one',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: expect.anything(),
|
||||
stitch_ticket: expect.anything(),
|
||||
},
|
||||
{
|
||||
entity_ref: 'k:ns/two',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: expect.anything(),
|
||||
stitch_ticket: expect.anything(),
|
||||
},
|
||||
]);
|
||||
|
||||
@@ -180,29 +180,29 @@ describe('markForStitching', () => {
|
||||
{
|
||||
entity_ref: 'k:ns/four',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: expect.anything(),
|
||||
stitch_ticket: expect.anything(),
|
||||
},
|
||||
{
|
||||
entity_ref: 'k:ns/one',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: expect.anything(),
|
||||
stitch_ticket: expect.anything(),
|
||||
},
|
||||
{
|
||||
entity_ref: 'k:ns/three',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: expect.anything(),
|
||||
stitch_ticket: expect.anything(),
|
||||
},
|
||||
{
|
||||
entity_ref: 'k:ns/two',
|
||||
next_stitch_at: expect.anything(),
|
||||
latest_ticket: expect.anything(),
|
||||
stitch_ticket: expect.anything(),
|
||||
},
|
||||
]);
|
||||
|
||||
// Entity 4's ticket should have been updated (was 'old', now something else)
|
||||
const final = await result();
|
||||
const entity4Final = final.find(r => r.entity_ref === 'k:ns/four');
|
||||
expect(entity4Final?.latest_ticket).not.toEqual('old');
|
||||
expect(entity4Final?.stitch_ticket).not.toEqual('old');
|
||||
},
|
||||
);
|
||||
|
||||
@@ -559,13 +559,13 @@ describe('markForStitching', () => {
|
||||
|
||||
// Verify final state - all entities should have been marked for stitching
|
||||
const finalState = await knex<DbStitchQueueRow>('stitch_queue')
|
||||
.select('entity_ref', 'next_stitch_at', 'latest_ticket')
|
||||
.select('entity_ref', 'next_stitch_at', 'stitch_ticket')
|
||||
.orderBy('entity_ref');
|
||||
|
||||
expect(finalState.length).toBeGreaterThan(0);
|
||||
finalState.forEach(row => {
|
||||
expect(row.next_stitch_at).not.toBeNull();
|
||||
expect(row.latest_ticket).not.toBeNull();
|
||||
expect(row.stitch_ticket).not.toBeNull();
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
@@ -92,12 +92,12 @@ export async function markForStitching(options: {
|
||||
.insert(
|
||||
chunk.map(ref => ({
|
||||
entity_ref: ref,
|
||||
latest_ticket: ticket,
|
||||
stitch_ticket: ticket,
|
||||
next_stitch_at: knex.fn.now(),
|
||||
})),
|
||||
)
|
||||
.onConflict('entity_ref')
|
||||
.merge(['next_stitch_at', 'latest_ticket']);
|
||||
.merge(['next_stitch_at', 'stitch_ticket']);
|
||||
}
|
||||
}, knex);
|
||||
}
|
||||
@@ -114,12 +114,12 @@ export async function markForStitching(options: {
|
||||
.insert(
|
||||
refreshStateRows.map(row => ({
|
||||
entity_ref: row.entity_ref,
|
||||
latest_ticket: ticket,
|
||||
stitch_ticket: ticket,
|
||||
next_stitch_at: knex.fn.now(),
|
||||
})),
|
||||
)
|
||||
.onConflict('entity_ref')
|
||||
.merge(['next_stitch_at', 'latest_ticket']);
|
||||
.merge(['next_stitch_at', 'stitch_ticket']);
|
||||
}
|
||||
}, knex);
|
||||
}
|
||||
|
||||
@@ -103,9 +103,9 @@ describe('performStitching', () => {
|
||||
stitchTicket: (
|
||||
await knex('stitch_queue')
|
||||
.where('entity_ref', 'k:ns/n')
|
||||
.select('latest_ticket')
|
||||
.select('stitch_ticket')
|
||||
.first()
|
||||
)?.latest_ticket,
|
||||
)?.stitch_ticket,
|
||||
});
|
||||
|
||||
entities = await knex<DbFinalEntitiesRow>('final_entities');
|
||||
@@ -200,9 +200,9 @@ describe('performStitching', () => {
|
||||
stitchTicket: (
|
||||
await knex('stitch_queue')
|
||||
.where('entity_ref', 'k:ns/n')
|
||||
.select('latest_ticket')
|
||||
.select('stitch_ticket')
|
||||
.first()
|
||||
)?.latest_ticket,
|
||||
)?.stitch_ticket,
|
||||
});
|
||||
|
||||
entities = await knex<DbFinalEntitiesRow>('final_entities');
|
||||
@@ -235,9 +235,9 @@ describe('performStitching', () => {
|
||||
stitchTicket: (
|
||||
await knex('stitch_queue')
|
||||
.where('entity_ref', 'k:ns/n')
|
||||
.select('latest_ticket')
|
||||
.select('stitch_ticket')
|
||||
.first()
|
||||
)?.latest_ticket,
|
||||
)?.stitch_ticket,
|
||||
});
|
||||
|
||||
entities = await knex<DbFinalEntitiesRow>('final_entities');
|
||||
|
||||
@@ -239,12 +239,12 @@ export async function performStitching(options: {
|
||||
.where('entity_id', entityId);
|
||||
|
||||
// In deferred mode, guard against concurrent stitchers by checking that
|
||||
// the latest_ticket in stitch_queue still matches what we were given.
|
||||
// the stitch_ticket in stitch_queue still matches what we were given.
|
||||
if (options.strategy.mode === 'deferred' && stitchTicket) {
|
||||
updateQuery = updateQuery.whereExists(
|
||||
knex<DbStitchQueueRow>('stitch_queue')
|
||||
.where('stitch_queue.entity_ref', entityRef)
|
||||
.where('stitch_queue.latest_ticket', stitchTicket)
|
||||
.where('stitch_queue.stitch_ticket', stitchTicket)
|
||||
.select(knex.raw('1')),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -102,6 +102,12 @@ describe('deleteOrphanedEntities', () => {
|
||||
.select('entity_ref', 'result_hash');
|
||||
}
|
||||
|
||||
async function stitchQueue(knex: Knex) {
|
||||
return await knex('stitch_queue')
|
||||
.orderBy('entity_ref')
|
||||
.select('entity_ref');
|
||||
}
|
||||
|
||||
async function finalEntities(knex: Knex) {
|
||||
return await knex<DbFinalEntitiesRow>('final_entities')
|
||||
.join(
|
||||
@@ -188,6 +194,7 @@ describe('deleteOrphanedEntities', () => {
|
||||
{ entity_ref: 'E8', result_hash: 'original' },
|
||||
{ entity_ref: 'E9', result_hash: 'original' },
|
||||
]);
|
||||
await expect(stitchQueue(knex)).resolves.toEqual([]);
|
||||
await expect(finalEntities(knex)).resolves.toEqual([
|
||||
{ entity_ref: 'E1', hash: 'original', next_stitch_at: null },
|
||||
{
|
||||
@@ -278,6 +285,10 @@ describe('deleteOrphanedEntities', () => {
|
||||
{ entity_ref: 'E8', result_hash: 'original' },
|
||||
{ entity_ref: 'E9', result_hash: 'original' },
|
||||
]);
|
||||
await expect(stitchQueue(knex)).resolves.toEqual([
|
||||
{ entity_ref: 'E2' },
|
||||
{ entity_ref: 'E7' },
|
||||
]);
|
||||
await expect(finalEntities(knex)).resolves.toEqual([
|
||||
{ entity_ref: 'E1', hash: 'original', next_stitch_at: null },
|
||||
{
|
||||
|
||||
@@ -156,16 +156,10 @@ export type DbStitchQueueRow = {
|
||||
/**
|
||||
* A random value that changes with every new stitch request. Used for
|
||||
* optimistic concurrency: when a stitch completes, the row is only deleted
|
||||
* if this ticket still matches the active_ticket (meaning no new request
|
||||
* came in while stitching was in progress).
|
||||
* if this ticket still matches (meaning no new request came in while
|
||||
* stitching was in progress).
|
||||
*/
|
||||
latest_ticket: string;
|
||||
/**
|
||||
* Set when a stitcher picks up this item for processing. The stitcher
|
||||
* compares this against latest_ticket before writing results; if they
|
||||
* differ, a new stitch request arrived and the current work is abandoned.
|
||||
*/
|
||||
active_ticket?: string | null;
|
||||
stitch_ticket: string;
|
||||
/**
|
||||
* The point in time when this entity should next be stitched.
|
||||
*
|
||||
@@ -181,7 +175,7 @@ export type DbStitchQueueRow = {
|
||||
* future.
|
||||
*
|
||||
* Only when a stitch run is completed successfully, AND it's found that the
|
||||
* latest ticket has not changed since the start (which means that no new
|
||||
* stitch ticket has not changed since the start (which means that no new
|
||||
* request has been made behind our backs), does the row get deleted.
|
||||
*/
|
||||
next_stitch_at: string | Date;
|
||||
|
||||
@@ -225,7 +225,6 @@ describe('DefaultLocationStore', () => {
|
||||
final_entity: '{}',
|
||||
hash: 'hash',
|
||||
last_updated_at: new Date(),
|
||||
|
||||
entity_ref: 'k:ns/n',
|
||||
});
|
||||
|
||||
|
||||
@@ -86,7 +86,6 @@ describe('GenericScmEventRefreshProvider', () => {
|
||||
entity_id: id,
|
||||
entity_ref: `k:ns/${id}`,
|
||||
hash: 'h',
|
||||
|
||||
final_entity: '{}',
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1005,18 +1005,18 @@ describe('migrations', () => {
|
||||
// Verify data was migrated correctly to stitch_queue
|
||||
const stitchQueueAfterUp = await knex('stitch_queue')
|
||||
.orderBy('entity_ref')
|
||||
.select('entity_ref', 'latest_ticket', 'next_stitch_at');
|
||||
.select('entity_ref', 'stitch_ticket', 'next_stitch_at');
|
||||
|
||||
// Only entities with pending stitches should be in stitch_queue (id1 and id3)
|
||||
expect(stitchQueueAfterUp).toEqual([
|
||||
{
|
||||
entity_ref: 'component:default/orphan-stitch',
|
||||
latest_ticket: 'ticket-3',
|
||||
stitch_ticket: 'ticket-3',
|
||||
next_stitch_at: expect.anything(),
|
||||
},
|
||||
{
|
||||
entity_ref: 'component:default/with-stitch',
|
||||
latest_ticket: 'ticket-1',
|
||||
stitch_ticket: 'ticket-1',
|
||||
next_stitch_at: expect.anything(),
|
||||
},
|
||||
]);
|
||||
|
||||
Reference in New Issue
Block a user