Skip to content

Commit

Permalink
Distinguish "missing repository" from "missing repository plugin" (el…
Browse files Browse the repository at this point in the history
…astic#82457)

Currently it is possible to uninstall repository plugin before
deleting repository that uses it. In such case the repository is still
listed in the API however every interaction with it produces the
"no such repository" exception. This is very confusing. This change 
updates the error message to correctly point the root of the issue.
  • Loading branch information
idegtiarenko authored Jan 14, 2022
1 parent 7018e9e commit 75579f7
Show file tree
Hide file tree
Showing 5 changed files with 327 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -140,7 +141,7 @@ public void registerRepository(final PutRepositoryRequest request, final ActionL

// Trying to create the new repository on master to make sure it works
try {
closeRepository(createRepository(newRepositoryMetadata, typesRegistry));
closeRepository(createRepository(newRepositoryMetadata, typesRegistry, this::throwRepositoryTypeDoesNotExists));
} catch (Exception e) {
listener.onFailure(e);
return;
Expand Down Expand Up @@ -490,7 +491,7 @@ public void applyClusterState(ClusterChangedEvent event) {
archiveRepositoryStats(repository, state.version());
repository = null;
try {
repository = createRepository(repositoryMetadata, typesRegistry);
repository = createRepository(repositoryMetadata, typesRegistry, this::createUnknownTypeRepository);
} catch (RepositoryException ex) {
// TODO: this catch is bogus, it means the old repo is already closed,
// but we have nothing to replace it
Expand All @@ -499,7 +500,7 @@ public void applyClusterState(ClusterChangedEvent event) {
}
} else {
try {
repository = createRepository(repositoryMetadata, typesRegistry);
repository = createRepository(repositoryMetadata, typesRegistry, this::createUnknownTypeRepository);
} catch (RepositoryException ex) {
logger.warn(() -> new ParameterizedMessage("failed to create repository [{}]", repositoryMetadata.name()), ex);
}
Expand Down Expand Up @@ -592,7 +593,7 @@ public void registerInternalRepository(String name, String type) {
RepositoryMetadata metadata = new RepositoryMetadata(name, type, Settings.EMPTY);
Repository repository = internalRepositories.computeIfAbsent(name, (n) -> {
logger.debug("put internal repository [{}][{}]", name, type);
return createRepository(metadata, internalTypesRegistry);
return createRepository(metadata, internalTypesRegistry, this::throwRepositoryTypeDoesNotExists);
});
if (type.equals(repository.getMetadata().type()) == false) {
logger.warn(
Expand Down Expand Up @@ -647,11 +648,15 @@ private void archiveRepositoryStats(Repository repository, long clusterStateVers
/**
* Creates repository holder. This method starts the repository
*/
private Repository createRepository(RepositoryMetadata repositoryMetadata, Map<String, Repository.Factory> factories) {
private Repository createRepository(
RepositoryMetadata repositoryMetadata,
Map<String, Repository.Factory> factories,
Function<RepositoryMetadata, Repository> defaultFactory
) {
logger.debug("creating repository [{}][{}]", repositoryMetadata.type(), repositoryMetadata.name());
Repository.Factory factory = factories.get(repositoryMetadata.type());
if (factory == null) {
throw new RepositoryException(repositoryMetadata.name(), "repository type [" + repositoryMetadata.type() + "] does not exist");
return defaultFactory.apply(repositoryMetadata);
}
Repository repository = null;
try {
Expand All @@ -668,6 +673,19 @@ private Repository createRepository(RepositoryMetadata repositoryMetadata, Map<S
}
}

private Repository throwRepositoryTypeDoesNotExists(RepositoryMetadata repositoryMetadata) {
throw new RepositoryException(repositoryMetadata.name(), "repository type [" + repositoryMetadata.type() + "] does not exist");
}

private Repository createUnknownTypeRepository(RepositoryMetadata repositoryMetadata) {
logger.warn(
"[{}] repository type [{}] is unknown; ensure that all required plugins are installed on this node",
repositoryMetadata.name(),
repositoryMetadata.type()
);
return new UnknownTypeRepository(repositoryMetadata);
}

private static void validate(final String repositoryName) {
if (Strings.hasLength(repositoryName) == false) {
throw new RepositoryException(repositoryName, "cannot be empty");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.repositories;

import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.ClusterStateUpdateTask;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.metadata.RepositoryMetadata;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.index.snapshots.IndexShardSnapshotStatus;
import org.elasticsearch.index.store.Store;
import org.elasticsearch.indices.recovery.RecoveryState;
import org.elasticsearch.snapshots.SnapshotId;

import java.io.IOException;
import java.util.Collection;
import java.util.function.Consumer;
import java.util.function.Function;

/**
* This class represents a repository that could not be initialized due to unknown type.
* This could happen whe a user creates a snapshot repository using a type from a plugin and then removes the plugin.
*/
public class UnknownTypeRepository extends AbstractLifecycleComponent implements Repository {

private final RepositoryMetadata repositoryMetadata;

public UnknownTypeRepository(RepositoryMetadata repositoryMetadata) {
this.repositoryMetadata = repositoryMetadata;
}

private RepositoryException createUnknownTypeException() {
return new RepositoryException(
repositoryMetadata.name(),
"repository type [" + repositoryMetadata.type() + "] is unknown; ensure that all required plugins are installed on this node"
);
}

@Override
public RepositoryMetadata getMetadata() {
return repositoryMetadata;
}

@Override
public void getSnapshotInfo(GetSnapshotInfoContext context) {
throw createUnknownTypeException();
}

@Override
public Metadata getSnapshotGlobalMetadata(SnapshotId snapshotId) {
throw createUnknownTypeException();
}

@Override
public IndexMetadata getSnapshotIndexMetaData(RepositoryData repositoryData, SnapshotId snapshotId, IndexId index) throws IOException {
throw createUnknownTypeException();
}

@Override
public void getRepositoryData(ActionListener<RepositoryData> listener) {
listener.onFailure(createUnknownTypeException());
}

@Override
public void finalizeSnapshot(FinalizeSnapshotContext finalizeSnapshotContext) {
finalizeSnapshotContext.onFailure(createUnknownTypeException());
}

@Override
public void deleteSnapshots(
Collection<SnapshotId> snapshotIds,
long repositoryStateId,
Version repositoryMetaVersion,
ActionListener<RepositoryData> listener
) {
listener.onFailure(createUnknownTypeException());
}

@Override
public long getSnapshotThrottleTimeInNanos() {
throw createUnknownTypeException();
}

@Override
public long getRestoreThrottleTimeInNanos() {
throw createUnknownTypeException();
}

@Override
public String startVerification() {
throw createUnknownTypeException();
}

@Override
public void endVerification(String verificationToken) {
throw createUnknownTypeException();
}

@Override
public void verify(String verificationToken, DiscoveryNode localNode) {
throw createUnknownTypeException();
}

@Override
public boolean isReadOnly() {
// this repository is assumed writable to bypass read-only check and fail with exception produced by this class
return false;
}

@Override
public void snapshotShard(SnapshotShardContext snapshotShardContext) {
snapshotShardContext.onFailure(createUnknownTypeException());
}

@Override
public void restoreShard(
Store store,
SnapshotId snapshotId,
IndexId indexId,
ShardId snapshotShardId,
RecoveryState recoveryState,
ActionListener<Void> listener
) {
listener.onFailure(createUnknownTypeException());
}

@Override
public IndexShardSnapshotStatus getShardSnapshotStatus(SnapshotId snapshotId, IndexId indexId, ShardId shardId) {
throw createUnknownTypeException();
}

@Override
public void updateState(ClusterState state) {

}

@Override
public void executeConsistentStateUpdate(
Function<RepositoryData, ClusterStateUpdateTask> createUpdateTask,
String source,
Consumer<Exception> onFailure
) {
onFailure.accept(createUnknownTypeException());
}

@Override
public void cloneShardSnapshot(
SnapshotId source,
SnapshotId target,
RepositoryShardId shardId,
ShardGeneration shardGeneration,
ActionListener<ShardSnapshotResult> listener
) {
listener.onFailure(createUnknownTypeException());
}

@Override
public void awaitIdle() {

}

@Override
protected void doStart() {

}

@Override
protected void doStop() {

}

@Override
protected void doClose() throws IOException {

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.cluster.ClusterChangedEvent;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.ClusterState;
Expand Down Expand Up @@ -50,7 +51,10 @@
import java.util.function.Consumer;
import java.util.function.Function;

import static org.elasticsearch.test.hamcrest.ThrowableAssertions.assertThatException;
import static org.elasticsearch.test.hamcrest.ThrowableAssertions.assertThatThrows;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.isA;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -162,6 +166,48 @@ public void testRepositoriesStatsCanHaveTheSameNameAndDifferentTypeOverTime() {
assertThat(repositoryStatsTypeB.getRepositoryStats(), equalTo(MeteredRepositoryTypeB.STATS));
}

// this can happen when the repository plugin is removed, but repository is still exist
public void testHandlesUnknownRepositoryTypeWhenApplyingClusterState() {
var repoName = randomAlphaOfLengthBetween(10, 25);

var clusterState = createClusterStateWithRepo(repoName, "unknown");
repositoriesService.applyClusterState(new ClusterChangedEvent("starting", clusterState, emptyState()));

var repo = repositoriesService.repository(repoName);
assertThat(repo, isA(UnknownTypeRepository.class));
}

public void testRemoveUnknownRepositoryTypeWhenApplyingClusterState() {
var repoName = randomAlphaOfLengthBetween(10, 25);

var clusterState = createClusterStateWithRepo(repoName, "unknown");
repositoriesService.applyClusterState(new ClusterChangedEvent("starting", clusterState, emptyState()));
repositoriesService.applyClusterState(new ClusterChangedEvent("removing repo", emptyState(), clusterState));

assertThatThrows(
() -> repositoriesService.repository(repoName),
RepositoryMissingException.class,
equalTo("[" + repoName + "] missing")
);
}

public void testRegisterRepositoryFailsForUnknownType() {
var repoName = randomAlphaOfLengthBetween(10, 25);
var request = new PutRepositoryRequest().name(repoName).type("unknown");

repositoriesService.registerRepository(request, new ActionListener<>() {
@Override
public void onResponse(AcknowledgedResponse acknowledgedResponse) {
fail("Should not register unknown repository type");
}

@Override
public void onFailure(Exception e) {
assertThatException(e, RepositoryException.class, equalTo("[" + repoName + "] repository type [unknown] does not exist"));
}
});
}

private ClusterState createClusterStateWithRepo(String repoName, String repoType) {
ClusterState.Builder state = ClusterState.builder(new ClusterName("test"));
Metadata.Builder mdBuilder = Metadata.builder();
Expand All @@ -179,8 +225,7 @@ private ClusterState emptyState() {
}

private void assertThrowsOnRegister(String repoName) {
PutRepositoryRequest request = new PutRepositoryRequest(repoName);
expectThrows(RepositoryException.class, () -> repositoriesService.registerRepository(request, null));
expectThrows(RepositoryException.class, () -> repositoriesService.registerRepository(new PutRepositoryRequest(repoName), null));
}

private static class TestRepository implements Repository {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.repositories;

import org.elasticsearch.cluster.metadata.RepositoryMetadata;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.test.ESTestCase;

public class UnknownTypeRepositoryTests extends ESTestCase {

private UnknownTypeRepository repository = new UnknownTypeRepository(new RepositoryMetadata("name", "type", Settings.EMPTY));

public void testShouldThrowWhenGettingMetadata() {
expectThrows(RepositoryException.class, () -> repository.getSnapshotGlobalMetadata(new SnapshotId("name", "uuid")));
}

public void testShouldNotThrowWhenApplyingLifecycleChanges() {
repository.start();
repository.stop();
}

public void testShouldNotThrowWhenClosingToAllowRemovingRepo() {
repository.close();
}
}
Loading

0 comments on commit 75579f7

Please sign in to comment.