Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions docs/testing/BINARY_S3_STORAGE.md

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ (Nit) Lines 15 and 18 still describe eviction as future work: "Cache eviction, which keeps that directory bounded, arrives in a later PR of this stack." and "Eviction, once it lands, deletes local files …". Eviction lands in this PR (BinaryCacheEvictionJob, documented under "Local cache eviction" below), so these two sentences should be updated here, e.g. "BinaryCacheEvictionJob keeps that directory bounded (see Local cache eviction)" and "Eviction deletes local files …".

Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ that mock `Config` statically must call `AssetStorageFeature.reset()` themselves
Turning the flag on is a one-way operational decision. With a namespace set (see below), the
objects written with the flag on live under `asset-namespaces/<namespace>/`, which a release
without this feature does not read. The flag-off path of this release ignores the namespace
too, so turning the flag off again hides those objects in the same way. The later slices that
store binaries in S3 add a layout that an older release cannot read either: a JSON reference at
the owner key and the bytes under `asset-blobs/sha256/...`. Once assets are written that way,
too, so turning the flag off again hides those objects in the same way. Binaries are stored in
a layout that an older release cannot read either: a JSON reference at the owner key and the
bytes under `asset-blobs/sha256/...`. Once assets are written that way,
do not roll back below the release that ships this feature without first migrating those
assets back to the previous layout.

Expand Down Expand Up @@ -88,11 +88,85 @@ The storage chain (`ChainableStoragePersistenceAPI`) and its providers change as
per lookup. Only listings that need every key (`listObjectPaths`, `listObjectSnapshots`,
`deleteGroup` and static push) follow every page.

Two building blocks are included for the slices that follow. `S3ContentAddressedStorage`
stores one immutable copy of each set of bytes at
`asset-blobs/sha256/<chars-1-2>/<chars-3-4>/<chars-5-6>/<chars-7-8>/<sha256>`.
`SharedExtractedMetadata` caches byte-derived Tika extraction at
`extracted-metadata/<source-sha256>/<configuration-hash>.json`. Nothing calls either one yet.
`extracted-metadata/<source-sha256>/<configuration-hash>.json`. Nothing calls it yet; metadata
generation starts using it in a later slice.

## Binary asset API

`BinaryAssetStorageAPI` (`APILocator.getBinaryAssetStorageAPI()`) manages binary assets and
completed renditions through the storage chain. Binary assets use the `binary-assets` group under
the asset root, laid out as `{inode[0]}/{inode[1]}/{inode}/{field}/{fileName}`. Renditions use the
`generated-assets` group under the `dotGenerated` root. With the flag on, keys in both groups keep
their mixed-case names; other groups are still lowercased.

With the flag on, S3 keeps one immutable copy of each set of bytes for these two groups, at
`asset-blobs/sha256/<chars-1-2>/<chars-3-4>/<chars-5-6>/<chars-7-8>/<sha256>`
(`S3ContentAddressedStorage`). The owned key holds a small reference object marked by the S3
user-metadata header `dotcms-blob-sha256`, and identical bytes under different names or owners
share one blob. Uploads hash a private snapshot, so the hashed bytes are the uploaded bytes.
Existing blobs are compared by content, and cold downloads verify SHA-256 before entering the
local cache. Older one-level blob keys and raw objects remain readable. Backfill replaces a raw
object with a reference only if its ETag still matches the version that was read, so a concurrent
replacement or deletion makes the migration fail instead of overwriting or resurrecting it.
Deleting an owner removes its reference, not the shared blob; shared blobs are not reclaimed yet.

With the flag on, a lookup by inode and field uses the binary reference recorded in the row's
`contentlet_as_json`. A row without that JSON (`SAVE_CONTENTLET_AS_JSON=false`, or a row not yet
populated) falls back to the older `{inode}/{field}/{fileName}` lookup instead of reporting no binary.

The API rejects an inode that is not at least two letters, digits, underscores or hyphens, and a
field or file name that is blank, contains `/` or `\`, or is `.` or `..`, so a built path cannot
leave its `{inode}/{field}` directory. This applies with the flag on or off.

With the flag off, the API uses the existing filesystem/NFS paths and makes no remote calls.

## Local cache eviction

With the flag on, the local asset directory is a cache that `BinaryCacheEvictionJob` can trim.
The job is scheduled only when `BINARY_CACHE_EVICTION_CRON` is set. Each node has its own cache, so
the job runs on every node from its own node-local thread (`binary-cache-eviction`), not from the
clustered Quartz scheduler, which would fire it on only one node per run. The first run is at least
ten minutes after startup, and each run schedules the next one when it finishes, so runs never
overlap. Because a run can take minutes, it does not use the shared scheduled pool. Once the
cache exceeds `BINARY_CACHE_MAX_SIZE_MB` (default 5000), it evicts the files least recently read by
anything first, with API reads always recorded, skipping files read or written within
`BINARY_CACHE_EVICTION_MIN_AGE_MINUTES` (default 60). The order comes from each file's access time.
The binary API sets the access time explicitly when it hands a cached file to a reader, at most once
a minute per file, so API reads count on `noatime` mounts too. Other reads count only where the
mount updates access times: on default `relatime` mounts the kernel also records a read once the
access time is over 24 hours old, which includes backup scans and the job's own verification of a
file it then keeps. The API does not touch the modification time, because HTTP ETags and
Last-Modified headers are built from it.

If a run ends with the cache still over its limit and no file evicted, the job logs a warning with
the number of candidates it kept.

A file is evicted only when S3 holds a verified copy under the exact key. For `binary-assets` and
`generated-assets`, the key must hold a reference whose SHA-256 matches the local file, and the
shared blob it names is then compared with the local file byte for byte. A raw object at the key,
written before references existed, is also compared byte for byte. For other groups the provider
requires the same size, then accepts a matching MD5 ETag and otherwise compares bytes. The byte comparison is deliberate: a matching
hash proves only what the reference says, not that the blob in the bucket still holds those bytes,
and eviction removes the last local copy. Its cost is one full download of each evicted file.

Only owned layouts are candidates (inode shards, a field/file pair or a UUID revision, or a
completed rendition). Operational files, hidden staging, temporary files and symlinked descendants
never enter the budget. Files without a verified S3 copy stay local, so the cache can exceed its
limit safely; eviction does not backfill.

Callers that hold a `File` across a read must take `acquireCacheLease()` before resolving it and
close the lease on the same thread after use. If a lease or storage operation is active when the job
reaches a file, verification is skipped entirely and the file is kept, so a busy node does not
download objects it cannot evict. Otherwise verification runs with no lock held, so readers never
wait for it. Only the final check and delete take the eviction lock, and they do not wait for it: if
a lease or storage operation is active at that moment, or the file changed during verification, the
file is kept for a later run. Sustained activity can therefore defer eviction on that node.

The lease is local to one process and does not coordinate external writers or several processes
sharing one asset directory. With the flag on, the binary API always uses the filesystem-plus-S3
chain, so the job evicts from whatever asset directory is configured, including a shared NFS mount.
Do not set `BINARY_CACHE_EVICTION_CRON` on nodes that share an asset directory.

## Credentials and region

Expand Down Expand Up @@ -178,7 +252,7 @@ docker run -d --rm --name binary-s3-test \
server /data

./mvnw test -pl :dotcms-core -Dmaven.build.cache.enabled=false \
-Dtest=AssetStorageFeatureTest,AssetStorageFeatureLatchTest,S3StorageConfigurationTest,NoWebIdentityCredentialsProviderChainTest,BinaryS3StorageTest \
-Dtest=AssetStorageFeatureTest,AssetStorageFeatureLatchTest,S3StorageConfigurationTest,NoWebIdentityCredentialsProviderChainTest,BinaryS3StorageTest,BinaryAssetReferenceTest,BinaryCacheEvictionJobTest,BinaryFileSystemStorageTest,BinaryAssetStorageAPIImplTest,MetadataLocalCacheTest \
-Ds3.test.endpoint=http://127.0.0.1:19002

docker stop binary-s3-test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.dotcms.enterprise.publishing.storage.AWSS3Storage;
import com.dotcms.enterprise.publishing.storage.Storage;
import com.dotcms.exception.ExceptionUtil;
import com.dotcms.storage.binary.BinaryAssetStorageAPI;
import com.dotcms.storage.repository.FileRepositoryManager;
import com.dotcms.storage.repository.HashedLocalFileRepositoryManager;
import com.dotcms.storage.repository.LocalFileRepositoryManager;
Expand Down Expand Up @@ -74,6 +75,13 @@ enum PathEncryptionMode {
private final Storage storage;
private final String namespace = configuredNamespace();

private boolean sharesBinaryBytes(final String group) {
Comment thread
fabrizzio-dotCMS marked this conversation as resolved.
return AssetStorageFeature.isEnabled() && pathEncryptionMode == PathEncryptionMode.NONE
&& (BinaryAssetStorageAPI.BINARY_ASSETS_GROUP.equals(group)
|| BinaryAssetStorageAPI.GENERATED_ASSETS_GROUP.equals(group));
}

private S3ContentAddressedStorage sharedBytes() { return new S3ContentAddressedStorage(storage, bucketName); }
private final FileRepositoryManager fileRepositoryManager = this.getFileRepository();
private MessageDigest sha256;
private final Set<String> groups = ConcurrentHashMap.newKeySet();
Expand Down Expand Up @@ -391,6 +399,9 @@ public List<String> listObjectPaths(final String groupName,
@EnterpriseFeature(licenseLevel = LicenseLevel.PLATFORM, errorMsg = INVALID_LICENSE)
public Object pushFile(final String groupName, final String path, final File file,
final Map<String, Serializable> extraMeta) throws DotDataException {
if (sharesBinaryBytes(groupName)) {
return sharedBytes().store(transformReadPath(groupName, path), file, false);
}
if (!AssetStorageFeature.isEnabled()) {
final String pathForS3 = transformWritePath(groupName, path, file.getName());
final Upload upload = this.storage.uploadFile(this.bucketName, pathForS3,
Expand Down Expand Up @@ -531,6 +542,13 @@ public File pullFile(final String groupName, final String path) throws DotDataEx
final File download = Try.of(() -> Files.createTempFile("s3-download-", ".tmp").toFile())
.getOrElseThrow(e -> new DotDataException("Unable to allocate an S3 download file", e));
try {
if (sharesBinaryBytes(groupName)) {
if (sharedBytes().retrieve(transformReadPath(groupName, path), download)) {
return download;
}
releaseRetrievedFile(download);
return null;
}
storage.downloadFile(bucketName, transformReadPath(groupName, path), download).waitForCompletion();
return download;
} catch (Exception e) {
Expand Down Expand Up @@ -801,6 +819,13 @@ public boolean hasDurableCopy(final String groupName, final String path,
return false;
}
final String key = transformReadPath(groupName, path);
if (sharesBinaryBytes(groupName)) {
try {
return sharedBytes().matches(key, file);
} catch (IOException | RuntimeException failure) {
throw new DotDataException("Unable to verify shared asset " + path, failure);
}
}
final var object = storage.listFirstObject(bucketName, key);
if (object == null || !key.equals(object.getKey()) || object.getSize() != file.length()) {
return false;
Expand Down Expand Up @@ -832,6 +857,10 @@ public boolean backfillFile(final String groupName, final String path, final Fil
if (!Files.isRegularFile(file.toPath(), java.nio.file.LinkOption.NOFOLLOW_LINKS)) {
throw new DotDataException("Backfill requires a regular source file: " + file);
}
if (sharesBinaryBytes(groupName)) {
sharedBytes().store(transformReadPath(groupName, path), file, true);
return true;
}
try {
if (hasDurableCopy(groupName, path, file)) {
return true;
Expand Down
49 changes: 41 additions & 8 deletions dotCMS/src/main/java/com/dotcms/storage/FileStorageAPIImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.dotmarketing.business.APILocator;
import com.dotmarketing.business.CacheLocator;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotRuntimeException;
import com.dotmarketing.portlets.contentlet.business.MetadataCache;
import com.dotmarketing.util.FileUtil;
import com.dotmarketing.util.Logger;
Expand All @@ -24,8 +25,10 @@
import com.liferay.util.StringPool;
import io.vavr.control.Try;
import java.io.File;
import java.io.IOException;
import java.io.Serializable;
import java.lang.ref.WeakReference;
import java.nio.file.NoSuchFileException;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
Expand Down Expand Up @@ -86,12 +89,12 @@ public FileStorageAPIImpl() {

@Override
public Map<String, Serializable> generateRawBasicMetaData(final File binary) {
return this.generateBasicMetaData(binary, s -> true); // raw = no filter
return withLocalBinary(binary, () -> this.generateBasicMetaData(binary, s -> true)); // raw = no filter
}

@Override
public Map<String, Serializable> generateRawFullMetaData(final File binary, long maxLength) {
return this.generateFullMetaData(binary, s -> true, maxLength); // raw = no filter
return withLocalBinary(binary, () -> this.generateFullMetaData(binary, s -> true, maxLength)); // raw = no filter
}

/**
Expand Down Expand Up @@ -158,8 +161,8 @@ private Map<String, Serializable> generateFullMetaData(final File binary,


} catch (Exception e) {
if (AssetStorageFeature.isEnabled() && e instanceof com.dotmarketing.exception.DotRuntimeException) {
throw (com.dotmarketing.exception.DotRuntimeException) e;
if (AssetStorageFeature.isEnabled() && e instanceof DotRuntimeException) {
throw (DotRuntimeException) e;
}
Logger.error(FileStorageAPIImpl.class, "Exception generating full metadata", e);
return Collections.emptyMap();
Expand Down Expand Up @@ -281,10 +284,10 @@ private Map<String, Serializable> tryRetrieveMetaData(final StorageKey storageKe

private Map<String, Serializable> generateMetadataFromFile(final File binary,
final GenerateMetadataConfig configuration, final StorageKey storageKey, final Map<String, Serializable> onlyHasCustomMetadataMap, final StoragePersistenceAPI storage) throws DotDataException {
validateBinary(binary);

final long maxLength = configuration.getMaxLength();
Map<String, Serializable> metadataMap = generateMetaDataBasedOnConfig(binary, configuration, maxLength);
Map<String, Serializable> metadataMap = withLocalBinary(binary, () -> {
validateBinary(binary);
return generateMetaDataBasedOnConfig(binary, configuration, configuration.getMaxLength());
});

if (configuration.isStore()) {
//if the group/bucket doesn't exist create it — only needed when actually writing.
Expand All @@ -295,6 +298,36 @@ private Map<String, Serializable> generateMetadataFromFile(final File binary,
return metadataMap;
}

/**
* Runs metadata generation with the binary present locally. With the flag on, the binary is
* restored from durable storage if it is not cached, and the cache lease is held through every
* file read.
*
* <p>A binary that is neither cached nor stored durably is simply absent, so generation runs
* as it does with the flag off: the raw generators return an empty result and the configured
* path rejects the file. A restore that fails is an error and is not hidden.</p>
*
* @param binary the binary to read
* @param generation the metadata generation to run
* @return the generated metadata
* @throws DotRuntimeException if the binary could not be restored
*/
private Map<String, Serializable> withLocalBinary(final File binary,
final Supplier<Map<String, Serializable>> generation) {
if (!AssetStorageFeature.isEnabled() || binary == null) {
return generation.get();
}
final var assets = APILocator.getBinaryAssetStorageAPI();
try (var lease = assets.acquireCacheLease(); var input = assets.openLocalFile(binary)) {
return generation.get();
} catch (final NoSuchFileException absent) {
return generation.get();
} catch (final IOException | DotDataException failure) {
throw new DotRuntimeException(
"Unable to restore binary for metadata: " + binary, failure);
}
}

private void validateBinary(final File binary) {
if (!validBinary(binary)) {
throw new IllegalArgumentException(String.format("the binary `%s` isn't accessible ",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.dotcms.concurrent.DotConcurrentFactory;
import com.dotcms.concurrent.lock.IdentifierStripedLock;
import com.dotcms.enterprise.achecker.parsing.EmptyIterable;
import com.dotcms.storage.binary.BinaryAssetStorageAPI;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotRuntimeException;
import com.dotmarketing.util.Config;
Expand Down Expand Up @@ -90,14 +91,18 @@ public void addGroupMapping(final String groupName, final File folder) {
}

/**
* Maps a storage key to its path on disk. Keys are lowercased, as they always have been.
* Maps a storage key to its path on disk. Keys are lowercased, as they always have been, except
* in the groups that keep mixed-case names while S3 asset storage is enabled.
*
* @param groupName the group the key belongs to
* @param path the storage key
* @return the path to use under the group's folder
*/
private String normalizePath(final String groupName, final String path) {
return path.toLowerCase();
return AssetStorageFeature.isEnabled()
&& (BinaryAssetStorageAPI.BINARY_ASSETS_GROUP.equals(groupName)
|| BinaryAssetStorageAPI.GENERATED_ASSETS_GROUP.equals(groupName))
? path : path.toLowerCase();
}

@Override
Expand Down
Loading
Loading