Adjust the length of blob cache docs for Lucene metadata files #69431

tlrx · 2021-02-23T11:41:19Z

Today searchable snapshots IndexInput implementations use the blob store cache to cache the first 4096 bytes of every Lucene files. After some experiments we think that we could adjust the length of the cached data depending of the Lucene file that is read, caching up to 64KB for Lucene metadata files (ie files that are fully read when a Directory is opened) and only 1KB for other files.

The files that are cached up to 64KB are the files with the following extensions:

    "cfe", // compound file's entry table
    "dvm", // doc values metadata file
    "fdm", // stored fields metadata file
    "fnm", // field names metadata file
    "kdm", // Lucene 8.6 point format metadata file
    "nvm", // norms metadata file
    "tmd", // Lucene 8.6 terms metadata file
    "tvm", // terms vectors metadata file
    "vem"  // Lucene 9.0 indexed vectors metadata

The 64KB limit can be configured on a per index basis through a new index setting.

This change is extracted from #69283 and does not address the caching of CFS files.

elasticmachine · 2021-02-23T11:41:22Z

Pinging @elastic/es-distributed (Team:Distributed)

tlrx · 2021-02-23T11:41:49Z

...Test/java/org/elasticsearch/blobstore/cache/SearchableSnapshotsBlobStoreCacheIntegTests.java

+        builder.put(SnapshotsService.SNAPSHOT_CACHE_REGION_SIZE_SETTING.getKey(), blobCacheMaxLength);
+        builder.put(SnapshotsService.SHARED_CACHE_RANGE_SIZE_SETTING.getKey(), blobCacheMaxLength);
+        builder.put(FrozenCacheService.FROZEN_CACHE_RECOVERY_RANGE_SIZE_SETTING.getKey(), blobCacheMaxLength);
+        cacheSettings = builder.build();


I'm trying to randomize these settings.

tlrx · 2021-02-23T11:42:33Z

...Test/java/org/elasticsearch/blobstore/cache/SearchableSnapshotsBlobStoreCacheIntegTests.java

+                builder.startObject().field("text", randomRealisticUnicodeOfCodepointLengthBetween(5, 50)).field("num", i).endObject();
+                indexRequestBuilders.add(client().prepareIndex(indexName).setSource(builder));
+            }
+            indexRandom(true, true, true, indexRequestBuilders);


It now uses dummyDocs in order to generate deletions

tlrx · 2021-02-23T11:43:26Z

...rchable-snapshots/src/main/java/org/elasticsearch/blobstore/cache/BlobStoreCacheService.java

+        if (METADATA_FILES_EXTENSIONS.contains(fileExtension)) {
+            final long maxAllowedLengthInBytes = maxMetadataLength.getBytes();
+            if (fileLength > maxAllowedLengthInBytes) {
+                logger.warn(


Maybe too verbose?

Let's use a Cache to log this once per filetype (and expire in an hour)

ywelsch

Thanks for opening this PR. I've done a first pass and left some comments.

ywelsch · 2021-02-23T12:48:17Z

...Test/java/org/elasticsearch/blobstore/cache/SearchableSnapshotsBlobStoreCacheIntegTests.java

-                if (indexInputStats.getTotalSize() <= BlobStoreCacheService.DEFAULT_CACHED_BLOB_SIZE * 2
-                    || mayReadMoreThanHeader == false) {
+                final boolean mayReadMoreThanHeader = indexInputStats.getFileExt().equals("cfs")
+                    || indexInputStats.getFileExt().equals("cfe");


cfs files I understand, but why can't we fully cache cfe files?

It's a leftover from debugging sessions, I removed them in 1d9cda0

ywelsch · 2021-02-23T12:52:19Z

...rchable-snapshots/src/main/java/org/elasticsearch/blobstore/cache/BlobStoreCacheService.java

@@ -41,7 +45,7 @@

    private static final Logger logger = LogManager.getLogger(BlobStoreCacheService.class);

-    public static final int DEFAULT_CACHED_BLOB_SIZE = ByteSizeUnit.KB.toIntBytes(4);
+    public static final int DEFAULT_CACHED_BLOB_SIZE = ByteSizeUnit.KB.toIntBytes(1);


How will this change affect documents in the blob cache that have been created with a previous ES version?

Sorry for the delayed response, I had to deal with other duties.

Looking at the current code in 7.x, reducing the size of cached blob might throw an exception in Cached/FrozenIndexInput at this line:

final BytesRefIterator cachedBytesIterator = cachedBlob.bytes().slice(toIntBytes(position), length).iterator();

in the case position is larger than one or two buffered reads.

I suggest

to keep the 4KB/8KB limit for non metadata files as long as there is at least a node in version < 7.13 in the cluster

to write some BWC test that ensure indices are correctly assigned during rolling upgrades

ywelsch · 2021-02-23T13:03:56Z

...rchable-snapshots/src/main/java/org/elasticsearch/blobstore/cache/BlobStoreCacheService.java

+        if (METADATA_FILES_EXTENSIONS.contains(fileExtension)) {
+            final long maxAllowedLengthInBytes = maxMetadataLength.getBytes();
+            if (fileLength > maxAllowedLengthInBytes) {
+                logger.warn(


Let's use a Cache to log this once per filetype (and expire in an hour)

...rchable-snapshots/src/main/java/org/elasticsearch/blobstore/cache/BlobStoreCacheService.java

...apshots/src/main/java/org/elasticsearch/index/store/cache/CachedBlobContainerIndexInput.java

...snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java

ywelsch · 2021-02-23T14:02:40Z

...Test/java/org/elasticsearch/blobstore/cache/SearchableSnapshotsBlobStoreCacheIntegTests.java

+
+        // Frozen (shared cache) cache should be large enough to not cause direct reads
+        builder.put(SnapshotsService.SNAPSHOT_CACHE_SIZE_SETTING.getKey(), ByteSizeValue.ofMb(128));
+        // Align ranges to match the blob cache max length


why have this alignment?

tlrx · 2021-03-01T08:16:48Z

Thanks @ywelsch for your review. I updated the code according to your comments.

Concerning #69431 (comment) and the behavior with previous versions, I followed my suggestion in #69431 (comment) and I added a rolling upgrade test named SearchableSnapshotsRollingUpgradeIT.

This integration test is composed of 4 tests:

two are tests that mount a snapshot as an index in the previous version and verifies that the index is recovered and searchable on mixed and upgraded version.
the two other tests are a bit tricky and try to test the behavior of the blob cache when cached blob documents are created from an old version or a new version. Those tests only runs in mixed version cluster. I validated that it throws the exception I suspected in Adjust the length of blob cache docs for Lucene metadata files #69431 (comment).

This is ready for another review.

ywelsch

LGTM. Thanks Tanguy!

tlrx · 2021-03-01T10:29:32Z

Thanks Yannick!

I'm going to backport this down to 7.12 and adjusts the BWC version OLD_CACHED_BLOB_SIZE_VERSION in follow up PRs.

…ic#69431) Today searchable snapshots IndexInput implementations use the blob store cache to cache the first 4096 bytes of every Lucene files. After some experiments we think that we could adjust the length of the cached data depending of the Lucene file that is read, caching up to 64KB for Lucene metadata files (ie files that are fully read when a Directory is opened) and only 1KB for other files. The files that are cached up to 64KB are the following extensions: "cfe", // compound file's entry table "dvm", // doc values metadata file "fdm", // stored fields metadata file "fnm", // field names metadata file "kdm", // Lucene 8.6 point format metadata file "nvm", // norms metadata file "tmd", // Lucene 8.6 terms metadata file "tvm", // terms vectors metadata file "vem" // Lucene 9.0 indexed vectors metadata The 64KB limit can be configured on a per index basis through a new index setting. This change is extracted from elastic#69283 and does not address the caching of CFS files. Backport of elastic#69431

Today searchable snapshots IndexInput implementations use the blob store cache to cache the first 4096 bytes of every Lucene files. After some experiments we think that we could adjust the length of the cached data depending of the Lucene file that is read, caching up to 64KB for Lucene metadata files (ie files that are fully read when a Directory is opened) and only 1KB for other files. The files that are cached up to 64KB are the following extensions: "cfe", // compound file's entry table "dvm", // doc values metadata file "fdm", // stored fields metadata file "fnm", // field names metadata file "kdm", // Lucene 8.6 point format metadata file "nvm", // norms metadata file "tmd", // Lucene 8.6 terms metadata file "tvm", // terms vectors metadata file "vem" // Lucene 9.0 indexed vectors metadata The 64KB limit can be configured on a per index basis through a new index setting. This change is extracted from #69283 and does not address the caching of CFS files. Backport of #69431

Adjust the length of blob cache docs for Lucene metadata files

b035670

tlrx added >enhancement :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 v7.13.0 labels Feb 23, 2021

elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Feb 23, 2021

tlrx commented Feb 23, 2021

View reviewed changes

revert cache range to 4KB

276838a

tlrx requested a review from ywelsch February 23, 2021 12:39

ywelsch added the v7.12.1 label Feb 23, 2021

ywelsch reviewed Feb 23, 2021

View reviewed changes

tlrx added 16 commits February 23, 2021 15:46

remove cfe

1d9cda0

add other types with assertions

40f7557

log only per hour

03fc26a

remove comment

0cc9e9d

remove assert on cached blob size

6e70ff3

reword TODO

5d28df8

remove NodeScope

b5f2463

Assume changed docs range as cache miss

a76a065

fix SearchableSnapshotDirectoryStatsTests

40fca31

Merge branch 'master' into adjust-cached-blob-size-for-metadata-files

73a055a

Merge branch 'master' into adjust-cached-blob-size-for-metadata-files

986f392

Merge branch 'master' into adjust-cached-blob-size-for-metadata-files

424dc2c

Add BWC support

68290e4

Merge branch 'master' into adjust-cached-blob-size-for-metadata-files

8942b93

Add BWC tests

36e1758

Merge branch 'master' into adjust-cached-blob-size-for-metadata-files

701a3a7

tlrx added 2 commits February 26, 2021 14:43

rename and fix tests (hopefully)

208fc4a

I hate you spotless

0894cd2

tlrx requested a review from ywelsch March 1, 2021 08:16

ywelsch approved these changes Mar 1, 2021

View reviewed changes

tlrx merged commit 5c2b15a into elastic:master Mar 1, 2021

tlrx deleted the adjust-cached-blob-size-for-metadata-files branch March 1, 2021 10:28

tlrx mentioned this pull request Mar 1, 2021

Adjust the length of blob cache docs for Lucene metadata files #69691

Merged

tlrx mentioned this pull request Mar 1, 2021

Adjust the length of blob cache docs for Lucene metadata files #69692

Merged

tlrx mentioned this pull request Mar 23, 2021

[Draft] Improve usage of blob store cache during searchable snapshots shard recovery #69283

Closed

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjust the length of blob cache docs for Lucene metadata files #69431

Adjust the length of blob cache docs for Lucene metadata files #69431

tlrx commented Feb 23, 2021

elasticmachine commented Feb 23, 2021

tlrx Feb 23, 2021

tlrx Feb 23, 2021

tlrx Feb 23, 2021

ywelsch Feb 23, 2021

ywelsch left a comment

ywelsch Feb 23, 2021

tlrx Feb 23, 2021

ywelsch Feb 23, 2021

tlrx Feb 25, 2021

ywelsch Feb 23, 2021

ywelsch Feb 23, 2021

tlrx commented Mar 1, 2021

ywelsch left a comment •

edited

Loading

tlrx commented Mar 1, 2021

Adjust the length of blob cache docs for Lucene metadata files #69431

Adjust the length of blob cache docs for Lucene metadata files #69431

Conversation

tlrx commented Feb 23, 2021

elasticmachine commented Feb 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywelsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlrx commented Mar 1, 2021

ywelsch left a comment • edited Loading

Choose a reason for hiding this comment

tlrx commented Mar 1, 2021

ywelsch left a comment •

edited

Loading