dotCMS / dotCMS/core

[spike] file asset creation and cleanup issues

Open
#36,858 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Area : Frontend Backend Bug Needs discussion
Dominant language
Java
Stars
970
Forks
486
Avg merge
3d 33m
Merged PRs (30d)
170

Description

This is not a finalized issue, rather a collection or related questions and issues that will require closer attention by a core dev.

tl;dr;
Will recently refactored 'Drop Old Asset' versions as it was not working on large-scale cloud envs. 'Clean Assets' is very old code and support is not comfortable running it in production. A deep dive with AI turned up a variety of potential interrelated issues.

Description

  1. Orphaned asset directories accumulate permanently. Empty directories are created for every contentlet version regardless of binary fields (A1). Non-transactional binary writes (A2), silent filesystem delete failures (A3), and DB-only deletes that skip filesystem cleanup (A4) all contribute additional orphans that no existing sweep can reach (A5). On NFS-backed storage this inflates directory counts by orders of magnitude.

  2. The clean-assets job is unsafe at production scale. A data-loss race condition deletes files for in-flight content (B1). One contentletAPI.find() per directory means millions of DB calls that also pollute the cache cluster-wide (B2). No throttling, batching, resume, or minimum-age floor (B3-B5). A legacy duplicate (CleanAssetsThread) is still wired to DWR with its own bugs (B7).

  3. Backend UI is broken and misleading. The asset-type dropdown is dead code — all three options do the same thing (D1). The new REST API with cancel/cluster-lock support has no UI consumer; the button still drives the worse legacy path (D2-D3). Descriptive text references checks that don't exist (D5). No dry-run, cancel, or run history exposed (D6). Need to ask; what should this UI finally look like?

Impacts

  • consuming considerable human bandwidth from our customers, support, ProServ, Cloud Eng see latest slack thread
  • empty directories add hours if not days to cloud env cloning tasks
  • support is reluctant to click 'Clean Assets' button
  • file asset management constraints are blockers for usage-based storage billing

Code Review Findings

Review of the asset cleanup code paths on main (3a3ed83). All file/line references are code-verified. Field validation status is called out per finding — a full filesystem audit has not been performed; the presence of a large number of empty directories under assets/ is confirmed.

A. Root causes of orphaned / unnecessary directories

A1 — An inode directory is created for every contentlet version of every type (confirmed in the field: empty directories)

ESContentletAPIImpl.handleBinaries() calls newDir.mkdirs() at ESContentletAPIImpl.java:6665, before the contentType.fields(BinaryField.class) loop, and handleBinaries is invoked unconditionally from checkin at ESContentletAPIImpl.java:6004 — there is no guard for content types that have no binary fields, and no early return inside the method.

Result: every version of every contentlet — blogs, pages, widgets, key/values — materializes an assets/<x>/<y>/<inode>/ directory that stays empty for its entire life. This is the direct explanation for the volume of empty directories observed. On NFS-backed storage this inflates directory count by orders of magnitude and slows every subsequent tree walk, including the cleanup jobs themselves.

Fix: move mkdirs() inside the per-field loop so the directory is created only when a file is actually written.

A2 — Binary writes are not transactional and have no rollback compensation

handleBinaries writes to the final asset path while the checkin transaction is still open: mkdirs() at :6665, then FileUtil.copyFile(...) at :6745-6759. A grep of the entire tree shows HibernateUtil.addRollbackListener is used only in WorkflowAPIImpl.java:1184 and CommitListenerCacheWrapper — nothing compensates binary writes.

Any rollback after :6004 leaves a fully-populated directory with no database row. Rollback sources downstream of that line include updatePublishAndExpireDates, host-field handling, permissionAPI.resetPermissionReferences, workflow actionlets, Elasticsearch index failures, and DB deadlock/statement timeout. handleBinaries itself throws on the zero-length-file check at :6693 after earlier fields in the same loop have already been copied.

Fix: stage binaries outside the final path and promote them via HibernateUtil.addCommitListener; register addRollbackListener to remove the new inode directory on failure.

A3 — Filesystem delete failures are silent

FileUtil.internalDelete() (com/liferay/util/FileUtil.java:365) swallows IOException at debug level. deltree() returns void; no caller inspects any result.

On NFS this is the dominant silent-failure mode. Deleting a file that another node or thread holds open triggers a server-side silly-rename to .nfsXXXXXXXX in the same directory: the file delete appears to succeed, the parent directory delete then fails with ENOTEMPTY, and the whole assets/<x>/<y>/<inode> tree survives with no log entry above debug. This fires precisely when content is deleted while a download or image resize is streaming the file. Stale file handles and uid-mapping EACCES fail the same way.

Fix: log at warn, have deltree return a status, and record failed paths for retry rather than dropping them.

A4 — Delete paths that touch the database and never the filesystem

  • ContentFileAssetIntegrityChecker.java:358,367
  • ContentPageIntegrityChecker.java:835,845
  • HostIntegrityChecker.java:494,505

These issue raw DELETE FROM contentlet WHERE identifier = ? AND inode = ? during push-publish integrity resolution with no corresponding deltree. Notably the same class relocates asset folders correctly in moveInodeFolder() (ContentFileAssetIntegrityChecker.java:586), so the omission on the delete path looks like an oversight rather than a design decision.

Also: FixTask00009CheckContentletsInexistentInodes.java:42 and FixTask00050FixInodesWithoutContentlets.java:42 delete rows and leave files.

A5 — Orphans created by A4 are permanently unreachable

deleteBinaryFiles(contentletsVersion, null) (ESContentletAPIImpl.java:9258) only removes directories for versions that findAllVersions(identifier) still returns. Any inode whose row was removed earlier by A4 can never be reached by a delete path again — only a filesystem sweep will ever find it. This is why the orphan set only grows.

A6 — deleteBinaryFiles runs inside the transaction

Called from destroy (:3119), delete (:3539, :3627) and deleteVersion (:3627), all under @WrapInTransaction. A rollback after the files are gone produces the inverse defect: rows present, files missing.

A7 — Resized-image tree is outside any sweep

getContentletCacheAssetPath() (ESContentletAPIImpl.java:9330) resolves to assets/cache/<x>/<y>/<inode>. deleteBinaryFiles clears it on the happy path, but the clean-assets sweep only walks single-hex-character directories at the assets root, so assets/cache is never examined. Same for dotGenerated (ImageFilter.java:208).

Note on reclaimed space: CONTENT_VERSION_HARD_LINK defaults to true (ESContentletAPIImpl.java:6745), so a version's file is a hardlink to the previous version's. Deleting one directory frees no space until every link is gone — orphan count and space reclaimed will not correlate.

Not a cause: temporary uploads. BinaryCleanupJob already reaps assets/tmp_upload, assets/bundles, dotsecure/trash, dotsecure/backup and java.io.tmpdir on a 3-hour / 3-4 day schedule. It does not touch assets/<x>/<y>.

B. "Clean Assets" job — unsafe at production scale

Two copies of the same algorithm exist: CleanAssetsJobProcessor.java:42 (job-queue wrapper, added in #35232) and the original CleanAssetsThread.java:120 (2012, still reachable via DWR at CMSMaintenanceAjax.java:265). Both perform a full 256-directory walk to count, a second full walk to clean, and one contentletAPI.find(inode) per directory.

  1. Data-loss race. No guard between "directory exists on disk" and "row visible in DB". Binaries are written before the transaction commits (A2), so a concurrent run sees no row and calls deleteQuietly on live content (CleanAssetsJobProcessor.java:199). There is no modification-time floor, no dry-run, and no report-only mode.
  2. One database round-trip per directory. Millions of find() calls, each loading a full contentlet through the API — which also populates the contentlet cache, evicting the working set cluster-wide for the duration of the run. Should be a batched SELECT inode FROM contentlet WHERE inode IN (...) via DotConnect, bypassing cache entirely.
  3. No throttling of any kind — no sleep, no batch size, no maximum runtime, no resume point. The sibling drop-old-versions job has all four.
  4. Double full traversal, materialising dir.list() / listFiles() arrays for potentially millions of entries per bucket. Should stream via Files.newDirectoryStream.
  5. Not resumable. Cancel or crash at 90% loses all position; no checkpoint of the last bucket processed.
  6. Directories only. !ff.isDirectory() → continue means legacy flat files assets/<x>/<y>/<inode>.<ext> are never reclaimed. Empty <x>/<y> buckets are never pruned either (we don't care about this)
  7. Legacy CleanAssetsThread is still wired to DWR — singleton, no cancel, no cluster lock, dir.list().length NPEs on an I/O error, and start() on a live thread throws IllegalThreadStateException. Two divergent copies to maintain; the comment at CleanAssetsJobProcessor.java:30 acknowledges this.
C. "Drop Old Asset Versions" — sound, with minor defects

DropOldContentVersionsJob.java:54 plus DropOldContentletRunner.java:117 are in good shape and are the right model for the clean-assets rewrite: batch size, inter-batch sleep, maximum runtime, dry-run flag, date-window iteration, automatic index creation, interrupt checks, and targeted per-inode deltree rather than a tree walk.

Defects found:

  • DropOldContentletRunner.java:143 — the loop condition endIterationDate < finalEndDate exits once the window clamps to finalEndDate, so the final DROP_OLD_ASSET_ITERATE_BY_DAYS slice before the cutoff is never processed.
  • DropOldContentVersionsJob.java:65-67 — javadoc says "Fire every Monday at 1:15AM"; the cron expression is 0 15 1 ? * WED *.
  • DropOldContentletRunner.java:50-52 — deletes from contentlet, inode and tag_inode only. ESContentFactoryImpl.deleteVersion additionally clears tree and multi-tree rows for the inode. The raw-SQL path appears to leave orphaned tree rows; worth confirming against the schema constraints.
  • DropOldContentletRunner.java:176-179 — the filesystem delete happens after conn.commit() with no retry. A crash or a failed deltree between the two leaves rows deleted and files behind (feeding A5).
  • CMSMaintenanceFactory.java:108 — the manual path ends in a cluster-wide CacheLocator.getCacheAdministrator().flushAll().
D. Backend UI

All of it is legacy Dojo/DWR in dotCMS/src/main/webapp/html/portlet/ext/cmsmaintenance/view_cms_maintenance.jsp (2003 lines).

  1. The Clean Assets dropdown is dead code. #whatClean with options all / binary / file_asset (JSP:1699-1703) is never read — doCleanAssets() at JSP:596 calls CMSMaintenanceAjax.cleanAssets(cb) with no argument, and CMSMaintenanceAjax.java:265 hardcodes getInstance(true, true). All three options do exactly the same thing. "Clean-only-fileasset" has never done what its label says.
  2. The new REST API has no consumer. MaintenanceResource.java:966 (POST /api/v1/maintenance/assets/_clean) and :1002 (status), backed by the job queue with cancel support and a cluster lock, are unreferenced anywhere in core-web. The JSP still drives the legacy singleton thread, so administrators hit the worse of the two implementations.
  3. The legacy path is not cluster-aware. Clicking the button runs the thread on that node only. No cancel, no persisted result, and a page reload orphans the UI while the thread keeps running. It polls DWR every second (JSP:628) for the entire run.
  4. The manual "Drop Old Assets Versions" control is redundant and riskier than the scheduled job. The job already runs weekly at 365 days. The button (doDropAssets, JSP:565) accepts an arbitrary date, runs the full windowed iteration synchronously inside the DWR request thread, and finishes with a cluster-wide cache flush.
  5. The descriptive text is inaccurate. "This process will check for asset inconsistencies before any old versions are removed" — DropOldContentletRunner performs no such check. Correspondingly, the removed == -2 branch at JSP:586 ("Database inconsistencies found. The process was cancelled") is unreachable; deleteOldAssetVersions returns only a count or -1.
  6. No dry-run, no cancel, no run history exposed, despite the job queue supporting all three.
E. Suggested direction

Ordered by leverage:

  1. Move newDir.mkdirs() inside the binary-field loop (A1). One line; stops the empty-directory growth that has already been observed.
  2. Stage binary writes and promote on commit, with a rollback listener to clean up (A2).
  3. Make filesystem deletes observable and retryable — warn-level logging, a return status, and a pending-deletion record (A3), which also bounds A4/A5.
  4. Add filesystem cleanup to the integrity checkers and fix tasks, or route them through ContentletAPI (A4).
  5. Rewrite clean-assets as a resumable, batched, throttled scanner modelled on DropOldContentletRunner: stream one <x>/<y> bucket at a time, checkpoint the bucket, batch existence checks with a single IN query per batch, never touch ContentletAPI or the cache, enforce a minimum-age floor, and default to report-only. Extend coverage to assets/cache and to pruning empty directories.
  6. Delete CleanAssetsThread and the DWR entry points; collapse the two UI rows into a single job panel driven by the job queue — last run, state, scanned/orphaned/deleted counts, cancel, dry-run toggle. The metadata CleanAssetsJobProcessor.getResultMetadata already returns covers most of it.
Validation status
  • Code-verified: every file/line reference above.
  • Field-confirmed: a large number of empty directories under assets/ (consistent with A1).
  • Not yet validated: no full filesystem audit has been run. The relative contribution of A2, A3 and A4 to the non-empty orphan population is unmeasured. Sampling orphaned directories and bucketing them — contains .nfs* (A3), empty (A1), holds a real binary (A2/A4) — would establish which root cause dominates before any of the deeper fixes are prioritised.
Desired Outcome
  • properly manage the full lifecycle of all files and directories for contentlet file in assets/
  • routine filesystem maintenance tasks should not require human interaction and should cover all anticipated issues and edge cases
  • improve UI/UX -- perhaps we have at most one "asset files cleanup" button in the backend
  • clear logging
Target Personas
  • Developer teams
  • Content teams
  • DevOps teams
  • System administrators (dotCMS)
Links

https://dotcms.slack.com/archives/C06TM536N9J/p1785511181741159

Backend UI
Image

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Begin with CleanAssetsJobProcessor.java, CleanAssetsThread.java, DropOldContentletRunner.java, and the maintenance JSP and REST entry points named in the issue. Trace how cleanup is scheduled, how filesystem and database operations interact, and how the UI invokes them. A complete effort would need an agreed scope, safe cleanup behavior, and an explicit replacement for the legacy UI path.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, javascript, sql
Domain
backend, database, devops, frontend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.