fix: Batch document deletes when emptying trash (#12328)

* fix: Batch document deletes when emptying trash

Splits the final parentDocumentId clear and destroy in documentPermanentDeleter
into chunks of 100 to keep the exclusive lock window on the documents table
short, preventing concurrent web SELECTs from queueing behind a single large
DELETE.

* fix: Skip parentDocumentId clear for documents restored mid-flight

Re-checks deletedAt in the database before clearing parentDocumentId on
children, so a parent restored between the caller's query and now keeps its
children attached.
This commit is contained in:
Tom Moor
2026-05-12 20:31:12 -04:00
committed by GitHub
parent 42a0958322
commit 6bd775eb84
2 changed files with 64 additions and 17 deletions
@@ -132,6 +132,27 @@ describe("documentPermanentDeleter", () => {
).toEqual(1);
});
it("should not detach children of a document restored between query and destroy", async () => {
const parent = await buildDocument({
publishedAt: subDays(new Date(), 90),
deletedAt: subDays(new Date(), 60),
});
const child = await buildDocument({
teamId: parent.teamId,
parentDocumentId: parent.id,
});
await Document.unscoped().update(
{ deletedAt: null },
{ where: { id: parent.id }, paranoid: false }
);
await documentPermanentDeleter([parent]);
await child.reload();
expect(child.parentDocumentId).toEqual(parent.id);
});
it("should not destroy attachments referenced in other documents", async () => {
const document1 = await buildDocument();
const document = await buildDocument({
+43 -17
View File
@@ -1,4 +1,4 @@
import { uniq } from "es-toolkit/compat";
import { chunk, uniq } from "es-toolkit/compat";
import { Op, QueryTypes } from "sequelize";
import Logger from "@server/logging/Logger";
import { Document, Attachment } from "@server/models";
@@ -76,26 +76,52 @@ export default async function documentPermanentDeleter(documents: Document[]) {
);
}
const documentIds = documents.map((document) => document.id);
await Document.update(
{
parentDocumentId: null,
},
{
where: {
parentDocumentId: {
[Op.in]: documentIds,
},
},
paranoid: false,
}
);
// Number of documents to delete per database statement. Keeps the exclusive
// lock window short enough to avoid blocking concurrent web requests that
// read from the documents table.
const BATCH_SIZE = 100;
return Document.scope("withDrafts").destroy({
const documentIds = documents.map((document) => document.id);
// Re-check deletedAt in the database to exclude documents that were restored
// between the caller's query and now. Otherwise the parentDocumentId clear
// below would detach children of a restored parent, breaking the hierarchy.
const stillDeleted = await Document.unscoped().findAll({
attributes: ["id"],
where: {
id: documentIds,
deletedAt: { [Op.ne]: null },
},
force: true,
paranoid: false,
});
const deletedIds = stillDeleted.map((document) => document.id);
const batches = chunk(deletedIds, BATCH_SIZE);
for (const batch of batches) {
await Document.update(
{
parentDocumentId: null,
},
{
where: {
parentDocumentId: {
[Op.in]: batch,
},
},
paranoid: false,
}
);
}
let totalDeleted = 0;
for (const batch of batches) {
totalDeleted += await Document.scope("withDrafts").destroy({
where: {
id: batch,
deletedAt: { [Op.ne]: null },
},
force: true,
});
}
return totalDeleted;
}