Bound the recursive folder walk for delete and copy: page a folder's contents, then decide on the transaction #37565

Description

Deleting a folder loads every contentlet in that folder into memory at once, and deletes the whole subtree inside one database transaction. Both are fine at the sizes most folders actually are, and neither is fine at the sizes some customers actually have.

This is deferred work from #37063, recorded here rather than left as a sentence in a spec. That ticket asks for a bulk folder delete and describes bounding this walk as "the real work"; the decision taken there was to ship the multi-select against today's delete and carry the bounding separately, on the grounds that it changes code every caller of folder deletion shares. The reasoning is in Additional Context so it is not re-derived.

This is not a Content Drive issue. The shipped single-folder delete (POST /api/v1/assets/folders/_delete, #35161) goes through exactly the same path and has exactly the same ceilings. Fixing it here fixes it for the context menu, for site deletion, and for anything in a customer plugin that calls FolderAPI.delete.

This ticket also covers folder copy (merged from #37769). FolderAPI.copy walks the tree the same way: recursive, one transaction for the whole subtree, and each folder's contents read in full. The Site Browser copy, plugins calling FolderAPI.copy and the Content Drive bulk duplicate (#37062) all go through it. Delete and copy should be bounded by one shared, paged folder walk rather than fixed twice. The copy specifics are in Copy: the same walk, with one difference below.

Two problems, and they are separable

That separability is the point of this ticket. They look like one thing — "the delete doesn't scale" — and they have different costs, different risks, and different blast radii.

Problem 1 can be fixed without touching problem 2. Paging the read does not move the transaction boundary, so the all-or-nothing guarantee is untouched and nothing downstream is renegotiated. Problem 2 cannot be fixed without giving that guarantee up.

Problem 1 — the contents are loaded whole, and nothing batches them

// FolderAPIImpl._deleteChildrenAssetsFromFolder, FolderAPIImpl.java:564
List<Contentlet> conList = capi.findContentletsByFolder(folder, user, false);
capi.destroy(conList, user, false);

findContentletsByFolder returns the folder's entire contents as one list, and destroy has no internal batching — verified. Before a single row is deleted, internalDestroy then walks that list twice more:

// ESContentletAPIImpl.internalDestroy
for (final Contentlet contentlet : contentlets) {
    final List<Contentlet> versions = this.findAllVersions(...);   // a query per contentlet
    for (final Contentlet version : versions) {
        this.canLock(version, user);                                // a check per version
    }
}

So a folder holding 100k files means 100k Contentlet objects plus all of their versions resident at once, and 100k+ queries before the first deletion happens. That is an out-of-memory failure on the node — it takes the JVM down for every other user, not just the one who pressed delete — and wrapping the call in a background job does nothing to help, because the memory is held by the call and not by the request.

Peak memory is bounded by the widest single folder, not by the depth of the tree. One flat media folder is enough; the subtree can be shallow.

Problem 2 — the subtree is one transaction

// FolderAPIImpl.java:427
@WrapInTransaction
public void delete(final Folder folder, final User user, final boolean respectFrontEndPermissions) {
    ...
    for (Folder childFolder : folderChildren) {
        delete(childFolder, user, respectFrontEndPermissions);   // :478 — re-enters the same transaction
    }

The recursion re-enters the same transaction, so deleting a folder with tens of thousands of descendants holds one transaction open for the whole run: locks retained, WAL accumulating, autovacuum blocked behind it. A failure late in a large delete rolls the whole thing back, and that rollback can take longer than the work it undoes.

The upside, which is why this half is the risky one to change: one transaction per folder is exactly what makes folder delete all-or-nothing today. A folder is never left half-deleted, under cancellation or under a crash — and deletion has no undo, so that guarantee is load-bearing rather than incidental. Splitting the transaction gives it up. That trade needs to be made deliberately, with the alternative (resume-on-restart, which the job framework does not provide) priced alongside it.

Note: today this guarantee holds for database rows only. File binaries are removed from disk while the delete runs, outside the transaction, so a delete that fails partway rolls back the rows but leaves the files it already reached deleted. That is tracked and being fixed in #37782 (remove from disk only after commit); any decision here should assume that fix is in place.

Copy: the same walk, with one difference

• FolderAPIImpl.copy is @WrapInTransaction, so one transaction covers the folder, every subfolder and everything copied into them.
• FolderFactoryImpl.saveCopiedFolder reads a folder's file assets (findFileAssetsByFolder), working pages (getWorkingHTMLPages) and links in full, with no paging, before copying them.
• The difference: copy's memory grows with the whole tree, not the widest folder. saveCopiedFolder passes one copiedObjects table through the whole recursion, and it keeps a pair of objects for every file, page and link copied anywhere in the tree. Paging each folder's read does not bound the copy until that table is removed or reduced to identifiers. It appears to be used only for containsKey checks.
• Walk order differs. Delete handles children before their parent; copy creates the parent before its children. The shared walk has to support both.
• Paging while deleting. Offset paging skips rows when the rows of the previous page have just been deleted. The walk should page by a sort key (or always take the first N remaining).
• Copy is milder than delete on the transaction side: it only reads the originals and inserts new rows, so it does not lock or remove anything other authors are using. Today a failed or cancelled copy leaves no partial duplicate, which Content Drive: folder copy, async job endpoint and frontend wiring #37062 relies on (specs/37062-folder-copy-backend/spec.md, D-012 leaves the copy as it is, D-013 states the all-or-nothing guarantee).

Proposed design: one shared folder walk

The walk changes how folders and their contents are read, not what any operation does. Every caller keeps its current behaviour: same items affected, same permission checks, same failures, same results. The walk is configured by the caller rather than deciding anything per operation.

FolderWalk.from(root)                       // Folder or Host
    .as(user, respectFrontEndPermissions)
    .requiring(PERMISSION_EDIT, PERMISSION_EDIT_PERMISSIONS)
    .onDenied(OnDenied.FAIL)
    .order(Order.CHILDREN_FIRST)            // PARENT_FIRST | CHILDREN_FIRST
    .visit(folder -> ...);

• Two pieces. The traversal reads subfolders a page at a time and checks permissions for each page in one call (permissionAPI.filterCollection), not per folder. The paged contents reader reads one folder's contents a page at a time. Delete and copy use both.
• Paging by sort key, not offset, so deleting the rows of one page does not make the next page skip items.
• Page size comes from Config, with a documented default.
• Bulk delete keeps its two levels. Across the selected folders it keeps going (FolderBulkDeleteProcessor records a failed folder and moves on); inside one folder, a refused subfolder stops that folder and rolls it back. The walk preserves both.

What is already decided, so it is not reopened here

• Content Drive: bulk folder delete, async job endpoint and frontend wiring #37063 ships against today's delete. Bulk folder delete wraps FolderAPI.delete unchanged. That removes the proxy timeout, which is the reported failure; it does not touch either ceiling above.
• The all-or-nothing guarantee stands for that feature, precisely because problem 2 is untouched.
• A follow-up is owed — this one.

What is not decided, and is the substance of this ticket, is whether either fix lives on the shared FolderAPI.delete that every caller uses, or on a second path. Changing the shared one is an API-behaviour change in the rollback-unsafe categories this repo tracks; adding a second one means folder delete behaves differently depending on how it was invoked, permanently. Neither is obviously right, and the answer may differ between the two halves.

Measure before building

Nobody has a number for where this actually breaks. "Large folder" is not a threshold anyone can act on, and both fixes need one — to size the work, to write a test that fails today, and to tell support what to warn about. That is the first acceptance criterion rather than an afterthought.

Acceptance Criteria

Measurement — do this first

• The folder size at which deleting one folder puts a node at risk is measured, not estimated, and recorded in the issue: contentlet count, resident memory, and wall-clock time, on a stated heap size
• The same is measured for transaction duration and lock retention on that folder
• An integration test reproduces the memory ceiling and fails on today's code

Problem 1 — page the contents

• A folder's contents are read and destroyed in bounded batches rather than loaded whole
• The batch size is configurable through Config, with a documented default
• Peak memory for deleting one folder is bounded by the batch size, not by the folder's width, demonstrated against the measurement above
• The transaction boundary is unchanged: one top-level folder is still one transaction, and a folder is still either fully deleted or untouched
• Nothing else changes — same contents destroyed, same permissions enforced, same events emitted, same behaviour for a folder small enough that batching never triggers

Problem 2 — decide, then either implement or record the decision

• A decision is recorded on whether the transaction is split, naming what replaces the all-or-nothing guarantee if it is, and what a partially deleted folder means for an operation with no undo
• If it is split: cancellation, process death and the abandoned-job sweep are each stated separately — they are three different interruptions and only the first is under the caller's control
• If it is not split: the reason is recorded here so the next reader does not re-derive it

Copy (merged from #37769)

• A folder's file assets, pages and links are read in pages rather than all at once, through the same walk delete uses
• Copy's peak memory no longer grows with the whole tree: the copiedObjects table is removed or holds identifiers only
• The transaction decision above states its outcome for copy as well: what a failed or cancelled copy leaves behind if split, and how the author is told
• The Site Browser copy, plugins calling FolderAPI.copy and the Content Drive bulk duplicate keep producing the same result for folders of ordinary size

Scope of the change

• For each half, it is stated and justified whether it lands on the shared FolderAPI.delete or on a separate path, and what that means for the shipped single-folder delete, site deletion, and customer plugins
• If the shared path changes, the rollback-safety labels required by docs/core/ROLLBACK_UNSAFE_CATEGORIES.md are applied

Shared walk

• Delete and copy go through one shared folder walk configured by root, user, required permissions, failure handling and order, as proposed above
• No caller changes behaviour: the existing integration tests for folder delete and copy pass unchanged

Tests

• Integration coverage for: a folder wide enough to exercise more than one batch, a folder small enough to exercise none, an interrupted delete leaving the folder untouched, and a permission refusal partway through (checking file binaries on disk as well as database rows), and the same wide, small and interrupted cases for copy
• New test classes are registered in a MainSuite* / Junit5Suite* @SuiteClasses list — an unregistered class compiles, passes locally, and is never run in CI
• Integration tests are run with -Dmaven.build.cache.enabled=false and Tests run: N is confirmed in target/failsafe-reports/*.txt rather than trusting the exit code

Priority

Medium

Additional Context

Why this was split out of #37063 rather than done there. Recording it so the decision is not relitigated:

• The defect Content Drive: bulk folder delete, async job endpoint and frontend wiring #37063 reports is that multi-select folder delete does not exist. Wrapping today's delete in a background job delivers that, and removes the proxy timeout along with it. Neither ceiling above is the reported failure.
• Bounding the walk means changing FolderAPIImpl / FolderFactoryImpl — code the context menu, site deletion and customer plugins all share. Doing that as a side effect of a Content Drive feature is how a rollback-unsafe regression reaches production wearing someone else's ticket number.
• As its own ticket it can be measured and tested against scale properly, and the shipped single-folder delete benefits too.

The two halves were deliberately deferred together so the cheap one is priced rather than assumed. If paging turns out to be small and safe, it is worth asking whether it belongs in #37063 after all — that is a judgement to make with an estimate in hand, and it is not a reason to reopen that ticket's scope decision.

Related findings in the same code, filed separately — do not fold them in here:

• The delete's contents lookup is permission-filtered and index-backed, while a later pass sweeps whatever it missed straight out of the database by path, with no permission check and no lock check. The narrower an author's rights, the less scrutiny the content under that folder receives.
• That same sweep removes content without recording anything that would remove it from the search index, and without the durable journal added by Content deletes are lost silently: index removal is fire-and-forget with no durable record, unlike adds #37276 — whose fix is scoped to destroyContentlets and does not reach this path. Every folder delete that reaches the sweep leaves orphaned search documents permanently.

Both are pre-existing, reachable through the shipped single-folder delete, and independent of this ticket.

Related: #37769 (folder copy, merged into this ticket), #37062 (bulk folder duplicate, the copy side was deferred from it), #37782 (folder delete loses binaries on a partway failure), #37063 (bulk folder delete — the ticket this was deferred from), #35161 (single-folder delete, shipped, same ceilings), #37276 (silent index delete loss), #33999 (parent epic).

Analysis and the two-path comparison behind the deferral: specs/37063-bulk-folder-delete-backend/spec.md (D-013, FR-020 … FR-023) and specs/37063-bulk-folder-delete-backend/drafts/b5-decision-paths.md.