Skip to content

Commit

Permalink
Simplify searchable snapshot CacheKey (#66263) (#66283)
Browse files Browse the repository at this point in the history
This commit simplifies the CacheKey object 
as suggested in #65725.

Backport of #66263 for 7.11
  • Loading branch information
tlrx authored Dec 14, 2020
1 parent abe37e0 commit 3c5d60e
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ protected IndexInputStats createIndexInputStats(final long fileLength) {
}

public CacheKey createCacheKey(String fileName) {
return new CacheKey(snapshotId, indexId, shardId, fileName);
return new CacheKey(snapshotId.getUUID(), indexId.getName(), shardId, fileName);
}

public CacheFile getCacheFile(CacheKey cacheKey, long fileLength) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,29 @@
package org.elasticsearch.index.store.cache;

import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.repositories.IndexId;
import org.elasticsearch.snapshots.SnapshotId;

import java.util.Objects;

public class CacheKey {

private final SnapshotId snapshotId;
private final IndexId indexId;
private final String snapshotUUID;
private final String snapshotIndexName;
private final ShardId shardId;
private final String fileName;

public CacheKey(SnapshotId snapshotId, IndexId indexId, ShardId shardId, String fileName) {
this.snapshotId = Objects.requireNonNull(snapshotId);
this.indexId = Objects.requireNonNull(indexId);
public CacheKey(String snapshotUUID, String snapshotIndexName, ShardId shardId, String fileName) {
this.snapshotUUID = Objects.requireNonNull(snapshotUUID);
this.snapshotIndexName = Objects.requireNonNull(snapshotIndexName);
this.shardId = Objects.requireNonNull(shardId);
this.fileName = Objects.requireNonNull(fileName);
}

public SnapshotId getSnapshotId() {
return snapshotId;
public String getSnapshotUUID() {
return snapshotUUID;
}

public IndexId getIndexId() {
return indexId;
public String getSnapshotIndexName() {
return snapshotIndexName;
}

public ShardId getShardId() {
Expand All @@ -50,19 +48,27 @@ public boolean equals(Object o) {
return false;
}
final CacheKey cacheKey = (CacheKey) o;
return Objects.equals(snapshotId, cacheKey.snapshotId)
&& Objects.equals(indexId, cacheKey.indexId)
return Objects.equals(snapshotUUID, cacheKey.snapshotUUID)
&& Objects.equals(snapshotIndexName, cacheKey.snapshotIndexName)
&& Objects.equals(shardId, cacheKey.shardId)
&& Objects.equals(fileName, cacheKey.fileName);
}

@Override
public int hashCode() {
return Objects.hash(snapshotId, indexId, shardId, fileName);
return Objects.hash(snapshotUUID, snapshotIndexName, shardId, fileName);
}

@Override
public String toString() {
return "[" + "snapshotId=" + snapshotId + ", indexId=" + indexId + ", shardId=" + shardId + ", fileName='" + fileName + "']";
return "[snapshotUUID="
+ snapshotUUID
+ ", snapshotIndexName="
+ snapshotIndexName
+ ", shardId="
+ shardId
+ ", fileName='"
+ fileName
+ "']";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ public void removeFromCache(final CacheKey cacheKey) {
* @param shardId the {@link SnapshotId}
*/
public void markShardAsEvictedInCache(SnapshotId snapshotId, IndexId indexId, ShardId shardId) {
final ShardEviction shardEviction = new ShardEviction(snapshotId, indexId, shardId);
final ShardEviction shardEviction = new ShardEviction(snapshotId.getUUID(), indexId.getName(), shardId);
if (evictedShards.add(shardEviction)) {
threadPool.generic().submit(new AbstractRunnable() {
@Override
Expand Down Expand Up @@ -380,7 +380,7 @@ public void onFailure(Exception e) {
* @param runnable a runnable to execute
*/
public void runIfShardMarkedAsEvictedInCache(SnapshotId snapshotId, IndexId indexId, ShardId shardId, Runnable runnable) {
runIfShardMarkedAsEvictedInCache(new ShardEviction(snapshotId, indexId, shardId), runnable);
runIfShardMarkedAsEvictedInCache(new ShardEviction(snapshotId.getUUID(), indexId.getName(), shardId), runnable);
}

/**
Expand Down Expand Up @@ -488,7 +488,9 @@ protected void synchronizeCache() {
assert value >= 0 : value;

final CacheKey cacheKey = cacheFile.getCacheKey();
if (evictedShards.contains(new ShardEviction(cacheKey.getSnapshotId(), cacheKey.getIndexId(), cacheKey.getShardId()))) {
if (evictedShards.contains(
new ShardEviction(cacheKey.getSnapshotUUID(), cacheKey.getSnapshotIndexName(), cacheKey.getShardId())
)) {
logger.debug("cache file belongs to a shard marked as evicted, skipping synchronization for [{}]", cacheKey);
continue;
}
Expand Down Expand Up @@ -579,13 +581,13 @@ public String toString() {
*/
private static class ShardEviction {

private final SnapshotId snapshotId;
private final IndexId indexId;
private final String snapshotUUID;
private final String snapshotIndexName;
private final ShardId shardId;

private ShardEviction(SnapshotId snapshotId, IndexId indexId, ShardId shardId) {
this.snapshotId = snapshotId;
this.indexId = indexId;
private ShardEviction(String snapshotUUID, String snapshotIndexName, ShardId shardId) {
this.snapshotUUID = snapshotUUID;
this.snapshotIndexName = snapshotIndexName;
this.shardId = shardId;
}

Expand All @@ -594,24 +596,24 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ShardEviction that = (ShardEviction) o;
return Objects.equals(snapshotId, that.snapshotId)
&& Objects.equals(indexId, that.indexId)
return Objects.equals(snapshotUUID, that.snapshotUUID)
&& Objects.equals(snapshotIndexName, that.snapshotIndexName)
&& Objects.equals(shardId, that.shardId);
}

@Override
public int hashCode() {
return Objects.hash(snapshotId, indexId, shardId);
return Objects.hash(snapshotUUID, snapshotIndexName, shardId);
}

@Override
public String toString() {
return "[snapshotId=" + snapshotId + ", indexId=" + indexId + ", shardId=" + shardId + ']';
return "[snapshotUUID=" + snapshotUUID + ", snapshotIndexName=" + snapshotIndexName + ", shardId=" + shardId + ']';
}

boolean matches(CacheKey cacheKey) {
return Objects.equals(snapshotId, cacheKey.getSnapshotId())
&& Objects.equals(indexId, cacheKey.getIndexId())
return Objects.equals(snapshotUUID, cacheKey.getSnapshotUUID())
&& Objects.equals(snapshotIndexName, cacheKey.getSnapshotIndexName())
&& Objects.equals(shardId, cacheKey.getShardId());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -486,9 +486,7 @@ public String toString() {
private static final String CACHE_PATH_FIELD = "cache_path";
private static final String CACHE_RANGES_FIELD = "cache_ranges";
private static final String SNAPSHOT_ID_FIELD = "snapshot_id";
private static final String SNAPSHOT_NAME_FIELD = "snapshot_name";
private static final String INDEX_ID_FIELD = "index_id";
private static final String INDEX_NAME_FIELD = "index_name";
private static final String SNAPSHOT_INDEX_NAME_FIELD = "index_name";
private static final String SHARD_INDEX_NAME_FIELD = "shard_index_name";
private static final String SHARD_INDEX_ID_FIELD = "shard_index_id";
private static final String SHARD_ID_FIELD = "shard_id";
Expand All @@ -503,10 +501,6 @@ private static String buildId(Path path) {
return path.getFileName().toString();
}

private static Term buildTerm(CacheFile cacheFile) {
return buildTerm(buildId(cacheFile));
}

private static Term buildTerm(String cacheFileUuid) {
return new Term(CACHE_ID_FIELD, cacheFileUuid);
}
Expand All @@ -530,14 +524,8 @@ private static Document buildDocument(NodeEnvironment.NodePath nodePath, CacheFi
final CacheKey cacheKey = cacheFile.getCacheKey();
document.add(new StringField(FILE_NAME_FIELD, cacheKey.getFileName(), Field.Store.YES));
document.add(new StringField(FILE_LENGTH_FIELD, Long.toString(cacheFile.getLength()), Field.Store.YES));

final SnapshotId snapshotId = cacheKey.getSnapshotId();
document.add(new StringField(SNAPSHOT_NAME_FIELD, snapshotId.getName(), Field.Store.YES));
document.add(new StringField(SNAPSHOT_ID_FIELD, snapshotId.getUUID(), Field.Store.YES));

final IndexId indexId = cacheKey.getIndexId();
document.add(new StringField(INDEX_NAME_FIELD, indexId.getName(), Field.Store.YES));
document.add(new StringField(INDEX_ID_FIELD, indexId.getId(), Field.Store.YES));
document.add(new StringField(SNAPSHOT_ID_FIELD, cacheKey.getSnapshotUUID(), Field.Store.YES));
document.add(new StringField(SNAPSHOT_INDEX_NAME_FIELD, cacheKey.getSnapshotIndexName(), Field.Store.YES));

final ShardId shardId = cacheKey.getShardId();
document.add(new StringField(SHARD_INDEX_NAME_FIELD, shardId.getIndex().getName(), Field.Store.YES));
Expand All @@ -555,8 +543,8 @@ private static String getValue(Document document, String fieldName) {

private static CacheKey buildCacheKey(Document document) {
return new CacheKey(
new SnapshotId(getValue(document, SNAPSHOT_NAME_FIELD), getValue(document, SNAPSHOT_ID_FIELD)),
new IndexId(getValue(document, INDEX_NAME_FIELD), getValue(document, INDEX_ID_FIELD)),
getValue(document, SNAPSHOT_ID_FIELD),
getValue(document, SNAPSHOT_INDEX_NAME_FIELD),
new ShardId(
new Index(getValue(document, SHARD_INDEX_NAME_FIELD), getValue(document, SHARD_INDEX_ID_FIELD)),
Integer.parseInt(getValue(document, SHARD_ID_FIELD))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.index.store.cache.CacheFile.EvictionListener;
import org.elasticsearch.index.store.cache.TestUtils.FSyncTrackingFileSystemProvider;
import org.elasticsearch.repositories.IndexId;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.ThreadPool;
import org.hamcrest.Matcher;
Expand Down Expand Up @@ -49,17 +47,12 @@
public class CacheFileTests extends ESTestCase {

private static final Runnable NOOP = () -> {};
private static final CacheKey CACHE_KEY = new CacheKey(
new SnapshotId("_name", "_uuid"),
new IndexId("_name", "_uuid"),
new ShardId("_name", "_uuid", 0),
"_filename"
);
private static final CacheKey CACHE_KEY = new CacheKey("_snap_uuid", "_snap_index", new ShardId("_name", "_uuid", 0), "_filename");

public void testGetCacheKey() throws Exception {
final CacheKey cacheKey = new CacheKey(
new SnapshotId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random())),
new IndexId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random())),
UUIDs.randomBase64UUID(random()),
randomAlphaOfLength(5).toLowerCase(Locale.ROOT),
new ShardId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random()), randomInt(5)),
randomAlphaOfLength(105).toLowerCase(Locale.ROOT)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,63 +5,56 @@
*/
package org.elasticsearch.index.store.cache;

import org.elasticsearch.common.UUIDs;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.repositories.IndexId;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.EqualsHashCodeTestUtils;

import java.util.Locale;

public class CacheKeyTests extends ESTestCase {

public void testEqualsAndHashCode() {
EqualsHashCodeTestUtils.checkEqualsAndHashCode(createInstance(), this::copy, this::mutate);
}

private SnapshotId randomSnapshotId() {
return new SnapshotId(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10));
private String randomSnapshotUUID() {
return UUIDs.randomBase64UUID(random());
}

private IndexId randomIndexId() {
return new IndexId(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10));
private String randomSnapshotIndexName() {
return randomAlphaOfLengthBetween(5, 10).toLowerCase(Locale.ROOT);
}

private ShardId randomShardId() {
return new ShardId(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10), randomInt(5));
}

private CacheKey createInstance() {
return new CacheKey(randomSnapshotId(), randomIndexId(), randomShardId(), randomAlphaOfLengthBetween(5, 10));
return new CacheKey(randomSnapshotUUID(), randomSnapshotIndexName(), randomShardId(), randomAlphaOfLengthBetween(5, 10));
}

private CacheKey copy(final CacheKey origin) {
SnapshotId snapshotId = origin.getSnapshotId();
if (randomBoolean()) {
snapshotId = new SnapshotId(origin.getSnapshotId().getName(), origin.getSnapshotId().getUUID());
}
IndexId indexId = origin.getIndexId();
if (randomBoolean()) {
indexId = new IndexId(origin.getIndexId().getName(), origin.getIndexId().getId());
}
ShardId shardId = origin.getShardId();
if (randomBoolean()) {
shardId = new ShardId(new Index(shardId.getIndex().getName(), shardId.getIndex().getUUID()), shardId.id());
}
return new CacheKey(snapshotId, indexId, shardId, origin.getFileName());
return new CacheKey(origin.getSnapshotUUID(), origin.getSnapshotIndexName(), shardId, origin.getFileName());
}

private CacheKey mutate(CacheKey origin) {
SnapshotId snapshotId = origin.getSnapshotId();
IndexId indexId = origin.getIndexId();
String snapshotUUID = origin.getSnapshotUUID();
String snapshotIndexName = origin.getSnapshotIndexName();
ShardId shardId = origin.getShardId();
String fileName = origin.getFileName();

switch (randomInt(3)) {
case 0:
snapshotId = randomValueOtherThan(origin.getSnapshotId(), this::randomSnapshotId);
snapshotUUID = randomValueOtherThan(snapshotUUID, this::randomSnapshotUUID);
break;
case 1:
indexId = randomValueOtherThan(origin.getIndexId(), this::randomIndexId);
snapshotIndexName = randomValueOtherThan(snapshotIndexName, this::randomSnapshotIndexName);
break;
case 2:
shardId = randomValueOtherThan(origin.getShardId(), this::randomShardId);
Expand All @@ -72,6 +65,6 @@ private CacheKey mutate(CacheKey origin) {
default:
throw new AssertionError("Unsupported mutation");
}
return new CacheKey(snapshotId, indexId, shardId, fileName);
return new CacheKey(snapshotUUID, snapshotIndexName, shardId, fileName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ public static void removeFileSystem() {
public void testCacheSynchronization() throws Exception {
final int numShards = randomIntBetween(1, 3);
final Index index = new Index(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random()));
final SnapshotId snapshotId = new SnapshotId("_snapshot_name", UUIDs.randomBase64UUID(random()));
final IndexId indexId = new IndexId("_index_name", UUIDs.randomBase64UUID(random()));
final String snapshotUUID = UUIDs.randomBase64UUID(random());
final String snapshotIndexName = UUIDs.randomBase64UUID(random());

logger.debug("--> creating shard cache directories on disk");
final Path[] shardsCacheDirs = new Path[numShards];
Expand All @@ -74,7 +74,7 @@ public void testCacheSynchronization() throws Exception {
assertFalse(Files.exists(shardDataPath));

logger.debug("--> creating directories [{}] for shard [{}]", shardDataPath.toAbsolutePath(), i);
shardsCacheDirs[i] = Files.createDirectories(resolveSnapshotCache(shardDataPath).resolve(snapshotId.getUUID()));
shardsCacheDirs[i] = Files.createDirectories(CacheService.resolveSnapshotCache(shardDataPath).resolve(snapshotUUID));
}

try (CacheService cacheService = defaultCacheService()) {
Expand Down Expand Up @@ -110,7 +110,7 @@ public void testCacheSynchronization() throws Exception {
for (int i = 0; i < between(1, 25); i++) {
final ShardId shardId = new ShardId(index, randomIntBetween(0, numShards - 1));
final String fileName = String.format(Locale.ROOT, "file_%d_%d", iteration, i);
final CacheKey cacheKey = new CacheKey(snapshotId, indexId, shardId, fileName);
final CacheKey cacheKey = new CacheKey(snapshotUUID, snapshotIndexName, shardId, fileName);
final CacheFile cacheFile = cacheService.get(cacheKey, randomIntBetween(0, 10_000), shardsCacheDirs[shardId.id()]);

final CacheFile.EvictionListener listener = evictedCacheFile -> {};
Expand Down Expand Up @@ -178,14 +178,14 @@ public void testPut() throws Exception {
try (CacheService cacheService = defaultCacheService()) {
final long fileLength = randomLongBetween(0L, 1000L);
final CacheKey cacheKey = new CacheKey(
new SnapshotId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random())),
new IndexId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random())),
UUIDs.randomBase64UUID(random()),
randomAlphaOfLength(5).toLowerCase(Locale.ROOT),
new ShardId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random()), randomInt(5)),
randomAlphaOfLength(105).toLowerCase(Locale.ROOT)
);

final Path cacheDir = Files.createDirectories(
resolveSnapshotCache(randomShardPath(cacheKey.getShardId())).resolve(cacheKey.getSnapshotId().getUUID())
resolveSnapshotCache(randomShardPath(cacheKey.getShardId())).resolve(cacheKey.getSnapshotUUID())
);
final String cacheFileUuid = UUIDs.randomBase64UUID(random());
final SortedSet<Tuple<Long, Long>> cacheFileRanges = randomBoolean() ? randomRanges(fileLength) : emptySortedSet();
Expand Down Expand Up @@ -238,7 +238,8 @@ public void testRunIfShardMarkedAsEvictedInCache() throws Exception {
final PlainActionFuture<Void> waitForEviction = PlainActionFuture.newFuture();
final CacheFile.EvictionListener evictionListener = evicted -> waitForEviction.onResponse(null);

final CacheFile cacheFile = cacheService.get(new CacheKey(snapshotId, indexId, shardId, "_0.dvd"), 100, cacheDir);
final CacheKey cacheKey = new CacheKey(snapshotId.getUUID(), indexId.getName(), shardId, "_0.dvd");
final CacheFile cacheFile = cacheService.get(cacheKey, 100, cacheDir);
cacheFile.acquire(evictionListener);

cacheService.markShardAsEvictedInCache(snapshotId, indexId, shardId);
Expand Down
Loading

0 comments on commit 3c5d60e

Please sign in to comment.