Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use blob store cache for Lucene compound files #69861

Merged
merged 8 commits into from
Mar 4, 2021

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Mar 3, 2021

The blob store cache is used to cache a variable length of the begining of Lucene files in the .snapshot-blob-cache system index. This is useful to speed up Lucene directory opening during shard recovery and to limit the number of bytes downloaded from the blob store when a searchable snapshot shard must be rebuilt.

This pull request adds support for compound files segment (.cfs) when they are partially cached (ie, Storage.SHARED_CACHE) so that the files they are composed of can also be cached in the blob store cache index.

Co-authored-by: Yannick Welsch yannick@welsch.lu

@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Mar 3, 2021
@elasticmachine
Copy link
Collaborator

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

@tlrx tlrx requested a review from ywelsch March 3, 2021 11:55
return Map.copyOf(blobsPerShard);
}

private void assertCachedBlobsInSystemIndex(final String repositoryName, final Map<String, BlobStoreIndexShardSnapshot> blobsInSnapshot)
Copy link
Member Author

Choose a reason for hiding this comment

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

In a follow up I'll add tests to verify the exact cached documents for CFS and non-CFS Lucene files, but for this integration test I think it is sufficient to verify that no bytes were downloaded after the second mount.

@@ -121,7 +121,7 @@ protected void doRun() throws Exception {
clone = indexInput.clone();
} else {
final int sliceEnd = between(readEnd, length);
clone = indexInput.slice("concurrent slice (0, " + sliceEnd + ") of " + indexInput, 0L, sliceEnd);
clone = indexInput.slice("concurrent slice" + randomFileExtension(), 0L, sliceEnd);
Copy link
Member Author

Choose a reason for hiding this comment

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

this is to better reflect what Lucene is doing when slicing a CFS file

final int length = b.remaining();

logger.trace("readInternal: read [{}-{}] ([{}] bytes) from [{}]", position, position + length, length, this);
try {
final CacheFile cacheFile = cacheFileReference.get();

// Can we serve the read directly from disk? If so, do so and don't worry about anything else.
if (isReadFromCompoundFileDuringRecovery(position, length) == false) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is to "force" the creation of cached blob docs during directory opening.

@tlrx
Copy link
Member Author

tlrx commented Mar 4, 2021

Build failure in elasticsearch-ci/1 is tracked in #69980. I'm waiting for this test to be muted and I'll rerun the tests.

@martijnvg
Copy link
Member

@tlrx I just muted it.

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

I've left one comment, o.w. looking good.

expectThrows(
IndexNotFoundException.class,
".snapshot-blob-cache system index should not be created yet",
() -> systemClient().admin().indices().prepareGetIndex().addIndices(SNAPSHOT_BLOB_CACHE_INDEX).get()
);

Storage storage = randomFrom(Storage.values());
// TODO randomize this with FULL_COPY too when cold tier also handle blob cache for footers
final Storage storage = Storage.SHARED_CACHE;
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer that we keep the cold storage variant here as well, and continue to assert about cfs further down below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense, I pusched 77828c9

@tlrx tlrx requested a review from ywelsch March 4, 2021 17:18
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

@tlrx tlrx merged commit 0cf97f7 into elastic:master Mar 4, 2021
@tlrx
Copy link
Member Author

tlrx commented Mar 4, 2021

Thanks for your help on this Yannick

@tlrx tlrx deleted the cache-cfs-in-blob-cache branch March 4, 2021 18:02
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Mar 4, 2021
The blob store cache is used to cache a variable length of the
begining of Lucene files in the .snapshot-blob-cache system
index. This is useful to speed up Lucene directory opening
during shard recovery and to limit the number of bytes
downloaded from the blob store when a searchable snapshot
shard must be rebuilt.

This commit adds support for compound files segment (.cfs)
when they are partially cached (ie, Storage.SHARED_CACHE)
so that the files they are composed of can also be cached in
the blob store cache index.

Co-Authored-By: Yannick Welsch <yannick@welsch.lu>
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Mar 4, 2021
The blob store cache is used to cache a variable length of the
begining of Lucene files in the .snapshot-blob-cache system
index. This is useful to speed up Lucene directory opening
during shard recovery and to limit the number of bytes
downloaded from the blob store when a searchable snapshot
shard must be rebuilt.

This commit adds support for compound files segment (.cfs)
when they are partially cached (ie, Storage.SHARED_CACHE)
so that the files they are composed of can also be cached in
the blob store cache index.

Co-Authored-By: Yannick Welsch <yannick@welsch.lu>
tlrx added a commit that referenced this pull request Mar 4, 2021
The blob store cache is used to cache a variable length of the
begining of Lucene files in the .snapshot-blob-cache system
index. This is useful to speed up Lucene directory opening
during shard recovery and to limit the number of bytes
downloaded from the blob store when a searchable snapshot
shard must be rebuilt.

This commit adds support for compound files segment (.cfs)
when they are partially cached (ie, Storage.SHARED_CACHE)
so that the files they are composed of can also be cached in
the blob store cache index.

Co-Authored-By: Yannick Welsch <yannick@welsch.lu>

Backport of #69861 for 7.12
tlrx added a commit that referenced this pull request Mar 4, 2021
The blob store cache is used to cache a variable length of the
begining of Lucene files in the .snapshot-blob-cache system
index. This is useful to speed up Lucene directory opening
during shard recovery and to limit the number of bytes
downloaded from the blob store when a searchable snapshot
shard must be rebuilt.

This commit adds support for compound files segment (.cfs)
when they are partially cached (ie, Storage.SHARED_CACHE)
so that the files they are composed of can also be cached in
the blob store cache index.

Co-Authored-By: Yannick Welsch <yannick@welsch.lu>

Backport of #69861 for 7.x
tlrx added a commit that referenced this pull request Mar 5, 2021
…atsTests (#70006)

Since #69861 CFS files read from FrozenIndexInput create 
dedicated frozen shared cache files when they are sliced. 
This does not play well with some tests that use the 
randomReadAndSlice to read files: this method can create 
overlapping slice/clone reads operations which makes it 
difficult to assert anything about CFS files with partial cache.

This commit prevent the tests to generate a .cfs file name 
when the partial cache is randomly picked up. As a follow 
up we should rework those test to make them more realistic 
with the new behavior.

Closes #70000
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Mar 5, 2021
…atsTests (elastic#70006)

Since elastic#69861 CFS files read from FrozenIndexInput create 
dedicated frozen shared cache files when they are sliced. 
This does not play well with some tests that use the 
randomReadAndSlice to read files: this method can create 
overlapping slice/clone reads operations which makes it 
difficult to assert anything about CFS files with partial cache.

This commit prevent the tests to generate a .cfs file name 
when the partial cache is randomly picked up. As a follow 
up we should rework those test to make them more realistic 
with the new behavior.

Closes elastic#70000
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Mar 5, 2021
…atsTests (elastic#70006)

Since elastic#69861 CFS files read from FrozenIndexInput create 
dedicated frozen shared cache files when they are sliced. 
This does not play well with some tests that use the 
randomReadAndSlice to read files: this method can create 
overlapping slice/clone reads operations which makes it 
difficult to assert anything about CFS files with partial cache.

This commit prevent the tests to generate a .cfs file name 
when the partial cache is randomly picked up. As a follow 
up we should rework those test to make them more realistic 
with the new behavior.

Closes elastic#70000
tlrx added a commit that referenced this pull request Mar 5, 2021
…atsTests (#70006) (#70019)

Since #69861 CFS files read from FrozenIndexInput create 
dedicated frozen shared cache files when they are sliced. 
This does not play well with some tests that use the 
randomReadAndSlice to read files: this method can create 
overlapping slice/clone reads operations which makes it 
difficult to assert anything about CFS files with partial cache.

This commit prevent the tests to generate a .cfs file name 
when the partial cache is randomly picked up. As a follow 
up we should rework those test to make them more realistic 
with the new behavior.

Closes #70000
tlrx added a commit that referenced this pull request Mar 5, 2021
…atsTests (#70006) (#70018)

Since #69861 CFS files read from FrozenIndexInput create 
dedicated frozen shared cache files when they are sliced. 
This does not play well with some tests that use the 
randomReadAndSlice to read files: this method can create 
overlapping slice/clone reads operations which makes it 
difficult to assert anything about CFS files with partial cache.

This commit prevent the tests to generate a .cfs file name 
when the partial cache is randomly picked up. As a follow 
up we should rework those test to make them more realistic 
with the new behavior.

Closes #70000
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.12.0 v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants