mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-08-19 02:51:47 +08:00
fix: history may duplicate on concurrency (#14487)
#### PR Dependency Tree * **PR #14487** 👈 This tree was auto-generated by [Charcoal](https://github.com/danerwilliams/charcoal) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Enhanced history record creation to prevent duplicate entries in concurrent scenarios. * **Tests** * Added validation for idempotent history record creation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -276,22 +276,16 @@ export class PgWorkspaceDocStorageAdapter extends DocStorageAdapter {
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
await this.models.history.create(
|
||||||
await this.models.history.create(
|
{
|
||||||
{
|
spaceId: snapshot.spaceId,
|
||||||
spaceId: snapshot.spaceId,
|
docId: snapshot.docId,
|
||||||
docId: snapshot.docId,
|
timestamp: snapshot.timestamp,
|
||||||
timestamp: snapshot.timestamp,
|
blob: Buffer.from(snapshot.bin),
|
||||||
blob: Buffer.from(snapshot.bin),
|
editorId: snapshot.editor,
|
||||||
editorId: snapshot.editor,
|
},
|
||||||
},
|
historyMaxAge
|
||||||
historyMaxAge
|
);
|
||||||
);
|
|
||||||
} catch (e) {
|
|
||||||
// safe to ignore
|
|
||||||
// only happens when duplicated history record created in multi processes
|
|
||||||
this.logger.error('Failed to create history record', e);
|
|
||||||
}
|
|
||||||
|
|
||||||
metrics.doc
|
metrics.doc
|
||||||
.counter('history_created_counter', {
|
.counter('history_created_counter', {
|
||||||
|
|||||||
@@ -74,6 +74,27 @@ test('should create a history record', async t => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('should not fail on duplicated history record', async t => {
|
||||||
|
const snapshot = {
|
||||||
|
spaceId: workspace.id,
|
||||||
|
docId: randomUUID(),
|
||||||
|
blob: Uint8Array.from([1, 2, 3]),
|
||||||
|
timestamp: Date.now(),
|
||||||
|
editorId: user.id,
|
||||||
|
};
|
||||||
|
|
||||||
|
const created1 = await t.context.history.create(snapshot, 1000);
|
||||||
|
const created2 = await t.context.history.create(snapshot, 1000);
|
||||||
|
t.deepEqual(created1.timestamp, snapshot.timestamp);
|
||||||
|
t.deepEqual(created2.timestamp, snapshot.timestamp);
|
||||||
|
|
||||||
|
const histories = await t.context.history.findMany(
|
||||||
|
snapshot.spaceId,
|
||||||
|
snapshot.docId
|
||||||
|
);
|
||||||
|
t.is(histories.length, 1);
|
||||||
|
});
|
||||||
|
|
||||||
test('should return null when history timestamp not match', async t => {
|
test('should return null when history timestamp not match', async t => {
|
||||||
const snapshot = {
|
const snapshot = {
|
||||||
spaceId: workspace.id,
|
spaceId: workspace.id,
|
||||||
|
|||||||
@@ -33,22 +33,33 @@ export class HistoryModel extends BaseModel {
|
|||||||
* Create a doc history with a max age.
|
* Create a doc history with a max age.
|
||||||
*/
|
*/
|
||||||
async create(snapshot: Doc, maxAge: number): Promise<DocHistorySimple> {
|
async create(snapshot: Doc, maxAge: number): Promise<DocHistorySimple> {
|
||||||
const row = await this.db.snapshotHistory.create({
|
const timestamp = new Date(snapshot.timestamp);
|
||||||
select: {
|
const expiredAt = new Date(Date.now() + maxAge);
|
||||||
timestamp: true,
|
|
||||||
createdByUser: { select: publicUserSelect },
|
// This method may be called concurrently by multiple processes for the same
|
||||||
|
// (workspaceId, docId, timestamp). Using upsert avoids duplicate key errors
|
||||||
|
// that would otherwise abort the surrounding transaction.
|
||||||
|
const row = await this.db.snapshotHistory.upsert({
|
||||||
|
where: {
|
||||||
|
workspaceId_id_timestamp: {
|
||||||
|
workspaceId: snapshot.spaceId,
|
||||||
|
id: snapshot.docId,
|
||||||
|
timestamp,
|
||||||
|
},
|
||||||
},
|
},
|
||||||
data: {
|
select: { timestamp: true, createdByUser: { select: publicUserSelect } },
|
||||||
|
create: {
|
||||||
workspaceId: snapshot.spaceId,
|
workspaceId: snapshot.spaceId,
|
||||||
id: snapshot.docId,
|
id: snapshot.docId,
|
||||||
timestamp: new Date(snapshot.timestamp),
|
timestamp,
|
||||||
blob: snapshot.blob,
|
blob: snapshot.blob,
|
||||||
createdBy: snapshot.editorId,
|
createdBy: snapshot.editorId,
|
||||||
expiredAt: new Date(Date.now() + maxAge),
|
expiredAt,
|
||||||
},
|
},
|
||||||
|
update: { expiredAt },
|
||||||
});
|
});
|
||||||
this.logger.debug(
|
this.logger.debug(
|
||||||
`Created history ${row.timestamp} for ${snapshot.docId} in ${snapshot.spaceId}`
|
`Upserted history ${row.timestamp} for ${snapshot.docId} in ${snapshot.spaceId}`
|
||||||
);
|
);
|
||||||
return {
|
return {
|
||||||
timestamp: row.timestamp.getTime(),
|
timestamp: row.timestamp.getTime(),
|
||||||
|
|||||||
Reference in New Issue
Block a user