From add8cbbba5dd3ad54142f4618eb7095e6b13a10c Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 25 Jun 2019 20:02:50 +0200 Subject: [PATCH 01/66] Repo cleanup endpoint start --- .../cleanup/CleanupRepositoryAction.java | 36 ++++++++++ .../cleanup/CleanupRepositoryRequest.java | 30 +++++++++ .../CleanupRepositoryRequestBuilder.java | 22 +++++++ .../cleanup/CleanupRepositoryResponse.java | 30 +++++++++ .../TransportCleanupRepositoryAction.java | 66 +++++++++++++++++++ 5 files changed, 184 insertions(+) create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java new file mode 100644 index 0000000000000..0209c902e9027 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java @@ -0,0 +1,36 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.action.admin.cluster.repositories.cleanup; + +import org.elasticsearch.action.Action; + +public final class CleanupRepositoryAction extends Action { + + public static final CleanupRepositoryAction INSTANCE = new CleanupRepositoryAction(); + public static final String NAME = "cluster:admin/repository/cleanup"; + + private CleanupRepositoryAction() { + super(NAME); + } + + @Override + public CleanupRepositoryResponse newResponse() { + return null; + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java new file mode 100644 index 0000000000000..dbc7966ad9bbe --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java @@ -0,0 +1,30 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.action.admin.cluster.repositories.cleanup; + +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.support.master.MasterNodeReadRequest; + +public class CleanupRepositoryRequest extends MasterNodeReadRequest { + + @Override + public ActionRequestValidationException validate() { + return null; + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java new file mode 100644 index 0000000000000..a0d37c50b5dad --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -0,0 +1,22 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.action.admin.cluster.repositories.cleanup; + +public class CleanupRepositoryRequestBuilder { +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java new file mode 100644 index 0000000000000..b9ced78137543 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java @@ -0,0 +1,30 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.action.admin.cluster.repositories.cleanup; + +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; + +public final class CleanupRepositoryResponse extends ActionResponse implements ToXContentObject { + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) { + return null; + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java new file mode 100644 index 0000000000000..d0eb7b372dc64 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -0,0 +1,66 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.action.admin.cluster.repositories.cleanup; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.master.TransportMasterNodeAction; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.block.ClusterBlockException; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; + +public final class TransportCleanupRepositoryAction extends TransportMasterNodeAction { + + private final RepositoriesService repositoriesService; + + @Override + protected String executor() { + return ThreadPool.Names.GENERIC; + } + + @Inject + public TransportCleanupRepositoryAction(TransportService transportService, ClusterService clusterService, + RepositoriesService repositoriesService, ThreadPool threadPool, ActionFilters actionFilters, + IndexNameExpressionResolver indexNameExpressionResolver) { + super(CleanupRepositoryAction.NAME, transportService, clusterService, threadPool, actionFilters, + indexNameExpressionResolver, CleanupRepositoryRequest::new); + this.repositoriesService = repositoriesService; + } + + @Override + protected CleanupRepositoryResponse newResponse() { + return null; + } + + @Override + protected void masterOperation(CleanupRepositoryRequest request, ClusterState state, + ActionListener listener) { + + } + + @Override + protected ClusterBlockException checkBlock(CleanupRepositoryRequest request, ClusterState state) { + return null; + } +} From 7abc87310b7f22f7ee6f4cfba579f3faf7d47e8b Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 26 Jun 2019 03:38:47 +0200 Subject: [PATCH 02/66] 50% --- .../elasticsearch/action/ActionModule.java | 3 ++ .../cleanup/CleanupRepositoryAction.java | 8 ++++- .../cleanup/CleanupRepositoryRequest.java | 35 ++++++++++++++++++- .../CleanupRepositoryRequestBuilder.java | 18 +++++++++- .../cleanup/CleanupRepositoryResponse.java | 34 ++++++++++++++++-- .../TransportCleanupRepositoryAction.java | 30 ++++++++++++---- .../client/ClusterAdminClient.java | 18 ++++++++++ .../client/support/AbstractClient.java | 19 ++++++++++ .../blobstore/BlobStoreRepository.java | 33 +++++++++++------ .../AbstractSnapshotIntegTestCase.java | 6 +--- .../DedicatedClusterSnapshotRestoreIT.java | 6 +--- 11 files changed, 179 insertions(+), 31 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 746ffd29213cc..f43aa71eddbb3 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -48,6 +48,8 @@ import org.elasticsearch.action.admin.cluster.node.usage.TransportNodesUsageAction; import org.elasticsearch.action.admin.cluster.remote.RemoteInfoAction; import org.elasticsearch.action.admin.cluster.remote.TransportRemoteInfoAction; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryAction; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.TransportCleanupRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.TransportDeleteRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesAction; @@ -443,6 +445,7 @@ public void reg actions.register(GetRepositoriesAction.INSTANCE, TransportGetRepositoriesAction.class); actions.register(DeleteRepositoryAction.INSTANCE, TransportDeleteRepositoryAction.class); actions.register(VerifyRepositoryAction.INSTANCE, TransportVerifyRepositoryAction.class); + actions.register(CleanupRepositoryAction.INSTANCE, TransportCleanupRepositoryAction.class); actions.register(GetSnapshotsAction.INSTANCE, TransportGetSnapshotsAction.class); actions.register(DeleteSnapshotAction.INSTANCE, TransportDeleteSnapshotAction.class); actions.register(CreateSnapshotAction.INSTANCE, TransportCreateSnapshotAction.class); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java index 0209c902e9027..fa711471f3260 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.cluster.repositories.cleanup; import org.elasticsearch.action.Action; +import org.elasticsearch.common.io.stream.Writeable; public final class CleanupRepositoryAction extends Action { @@ -31,6 +32,11 @@ private CleanupRepositoryAction() { @Override public CleanupRepositoryResponse newResponse() { - return null; + return new CleanupRepositoryResponse(0L, 0L); + } + + @Override + public Writeable.Reader getResponseReader() { + return CleanupRepositoryResponse::new; } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java index dbc7966ad9bbe..c9c6e19ca7ebd 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java @@ -20,11 +20,44 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.support.master.MasterNodeReadRequest; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; + +import java.io.IOException; + +import static org.elasticsearch.action.ValidateActions.addValidationError; public class CleanupRepositoryRequest extends MasterNodeReadRequest { + private String repository; + + public CleanupRepositoryRequest(String repository) { + this.repository = repository; + } + + public CleanupRepositoryRequest(StreamInput in) throws IOException { + repository = in.readString(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(repository); + } + @Override public ActionRequestValidationException validate() { - return null; + ActionRequestValidationException validationException = null; + if (repository == null) { + validationException = addValidationError("repository is null", null); + } + return validationException; + } + + public String repository() { + return repository; + } + + public void repository(String repository) { + this.repository = repository; } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java index a0d37c50b5dad..002c7c0aa05f2 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -18,5 +18,21 @@ */ package org.elasticsearch.action.admin.cluster.repositories.cleanup; -public class CleanupRepositoryRequestBuilder { +import org.elasticsearch.action.Action; +import org.elasticsearch.action.support.master.MasterNodeReadOperationRequestBuilder; +import org.elasticsearch.client.ElasticsearchClient; + +public class CleanupRepositoryRequestBuilder extends MasterNodeReadOperationRequestBuilder { + + public CleanupRepositoryRequestBuilder(ElasticsearchClient client, Action action, + String repository) { + super(client, action, new CleanupRepositoryRequest(repository)); + } + + public CleanupRepositoryRequestBuilder setRepository(String repository) { + request.repository(repository); + return this; + } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java index b9ced78137543..3bb3a0efd55cb 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java @@ -19,12 +19,42 @@ package org.elasticsearch.action.admin.cluster.repositories.cleanup; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; +import java.io.IOException; + public final class CleanupRepositoryResponse extends ActionResponse implements ToXContentObject { + + private final long bytes; + + private final long blobs; + + public CleanupRepositoryResponse(long bytes, long blobs) { + this.bytes = bytes; + this.blobs = blobs; + } + + public CleanupRepositoryResponse(StreamInput in) throws IOException { + bytes = in.readLong(); + blobs = in.readLong(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeLong(bytes); + out.writeLong(blobs); + } + + @Override + public void readFrom(StreamInput in) { + throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); + } + @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) { - return null; + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return builder.startObject().field("bytes", bytes).field("blobs", blobs).endObject(); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index d0eb7b372dc64..bfa66c068f64e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -20,17 +20,24 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.master.TransportMasterNodeAction; +import org.elasticsearch.action.support.master.TransportMasterNodeReadAction; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; +import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.repositories.Repository; +import org.elasticsearch.repositories.blobstore.BlobStoreRepository; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -public final class TransportCleanupRepositoryAction extends TransportMasterNodeAction { +import java.io.IOException; + +public final class TransportCleanupRepositoryAction extends TransportMasterNodeReadAction { private final RepositoriesService repositoriesService; @@ -44,23 +51,34 @@ public TransportCleanupRepositoryAction(TransportService transportService, Clust RepositoriesService repositoriesService, ThreadPool threadPool, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) { super(CleanupRepositoryAction.NAME, transportService, clusterService, threadPool, actionFilters, - indexNameExpressionResolver, CleanupRepositoryRequest::new); + CleanupRepositoryRequest::new, indexNameExpressionResolver); this.repositoriesService = repositoriesService; } @Override protected CleanupRepositoryResponse newResponse() { - return null; + return new CleanupRepositoryResponse(0L, 0L); + } + + @Override + protected CleanupRepositoryResponse read(StreamInput in) throws IOException { + return new CleanupRepositoryResponse(in); } @Override protected void masterOperation(CleanupRepositoryRequest request, ClusterState state, ActionListener listener) { - + Repository repository = repositoriesService.repository(request.repository()); + if (repository instanceof BlobStoreRepository) { + ((BlobStoreRepository) repository).cleanup(repository.getRepositoryData().getGenId(), listener); + } else { + throw new IllegalArgumentException("Repository [" + request.repository()+ "] is not a blob store repository"); + } } @Override protected ClusterBlockException checkBlock(CleanupRepositoryRequest request, ClusterState state) { - return null; + // Cluster is not affected but we look up repositories in metadata + return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ); } } diff --git a/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java b/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java index cd874b62a40b4..fdee39fdb1f93 100644 --- a/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java +++ b/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java @@ -49,6 +49,9 @@ import org.elasticsearch.action.admin.cluster.node.usage.NodesUsageRequest; import org.elasticsearch.action.admin.cluster.node.usage.NodesUsageRequestBuilder; import org.elasticsearch.action.admin.cluster.node.usage.NodesUsageResponse; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequestBuilder; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequestBuilder; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesRequest; @@ -453,6 +456,21 @@ public interface ClusterAdminClient extends ElasticsearchClient { */ GetRepositoriesRequestBuilder prepareGetRepositories(String... name); + /** + * Cleans up repository. + */ + CleanupRepositoryRequestBuilder prepareCleanupRepository(String repository); + + /** + * Cleans up repository. + */ + ActionFuture cleanupRepository(CleanupRepositoryRequest repository); + + /** + * Cleans up repository. + */ + void cleanupRepository(CleanupRepositoryRequest repository, ActionListener listener); + /** * Verifies a repository. */ diff --git a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java index fad9e97e9ed08..dfdda9f76088a 100644 --- a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java +++ b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java @@ -64,6 +64,10 @@ import org.elasticsearch.action.admin.cluster.node.usage.NodesUsageRequest; import org.elasticsearch.action.admin.cluster.node.usage.NodesUsageRequestBuilder; import org.elasticsearch.action.admin.cluster.node.usage.NodesUsageResponse; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryAction; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequestBuilder; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequestBuilder; @@ -1004,6 +1008,21 @@ public GetRepositoriesRequestBuilder prepareGetRepositories(String... name) { return new GetRepositoriesRequestBuilder(this, GetRepositoriesAction.INSTANCE, name); } + @Override + public CleanupRepositoryRequestBuilder prepareCleanupRepository(String repository) { + return new CleanupRepositoryRequestBuilder(this, CleanupRepositoryAction.INSTANCE, repository); + } + + @Override + public ActionFuture cleanupRepository(CleanupRepositoryRequest request) { + return execute(CleanupRepositoryAction.INSTANCE, request); + } + + @Override + public void cleanupRepository(CleanupRepositoryRequest request, ActionListener listener) { + execute(CleanupRepositoryAction.INSTANCE, request, listener); + } + @Override public ActionFuture restoreSnapshot(RestoreSnapshotRequest request) { return execute(RestoreSnapshotAction.INSTANCE, request); diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index bea772eb8826e..91cc5c4057e55 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -32,6 +32,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRunnable; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.support.GroupedActionListener; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaData; @@ -399,7 +400,7 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action updatedRepositoryData = repositoryData.removeSnapshot(snapshotId); // Cache the indices that were found before writing out the new index-N blob so that a stuck master will never // delete an index that was created by another master node after writing this index-N blob. - foundIndices = blobStore().blobContainer(basePath().add("indices")).children(); + foundIndices = blobStore().blobContainer(indicesPath()).children(); writeIndexGen(updatedRepositoryData, repositoryStateId); } catch (Exception ex) { listener.onFailure(new RepositoryException(metadata.name(), "failed to delete snapshot [" + snapshotId + "]", ex)); @@ -427,6 +428,15 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action } } + public void cleanup(long repositoryStateId, ActionListener listener) { + ActionListener.completeWith(listener, () -> { + final Map foundIndices = blobStore().blobContainer(indicesPath()).children(); + final RepositoryData repositoryData = repositoryData(repositoryStateId); + cleanupStaleIndices(foundIndices, repositoryData.getIndices()); + return new CleanupRepositoryResponse(0L, 0L); + }); + } + private void cleanupStaleIndices(Map foundIndices, Map survivingIndices) { try { final Set survivingIndexIds = survivingIndices.values().stream() @@ -641,7 +651,16 @@ public void endVerification(String seed) { @Override public RepositoryData getRepositoryData() { try { - final long indexGen = latestIndexBlobId(); + return repositoryData(latestIndexBlobId()); + } catch (NoSuchFileException ex) { + // repository doesn't have an index blob, its a new blank repo + return RepositoryData.EMPTY; + } catch (IOException ioe) { + throw new RepositoryException(metadata.name(), "could not read repository data from index blob", ioe); + } + } + + private RepositoryData repositoryData(long indexGen) throws IOException { final String snapshotsIndexBlobName = INDEX_FILE_PREFIX + Long.toString(indexGen); RepositoryData repositoryData; @@ -669,8 +688,8 @@ public RepositoryData getRepositoryData() { } catch (NoSuchFileException e) { if (isReadOnly()) { logger.debug("[{}] Incompatible snapshots blob [{}] does not exist, the likely " + - "reason is that there are no incompatible snapshots in the repository", - metadata.name(), INCOMPATIBLE_SNAPSHOTS_BLOB); + "reason is that there are no incompatible snapshots in the repository", + metadata.name(), INCOMPATIBLE_SNAPSHOTS_BLOB); } else { // write an empty incompatible-snapshots blob - we do this so that there // is a blob present, which helps speed up some cloud-based repositories @@ -680,12 +699,6 @@ public RepositoryData getRepositoryData() { } } return repositoryData; - } catch (NoSuchFileException ex) { - // repository doesn't have an index blob, its a new blank repo - return RepositoryData.EMPTY; - } catch (IOException ioe) { - throw new RepositoryException(metadata.name(), "could not read repository data from index blob", ioe); - } } private static String testBlobPrefix(String seed) { diff --git a/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java b/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java index 9f680f11f2117..ebe6caaf8fd97 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java +++ b/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java @@ -76,11 +76,7 @@ public void assertRepoConsistency() { .stream() .map(RepositoryMetaData::name) .forEach(name -> { - final List snapshots = client().admin().cluster().prepareGetSnapshots(name).get().getSnapshots(name); - // Delete one random snapshot to trigger repository cleanup. - if (snapshots.isEmpty() == false) { - client().admin().cluster().prepareDeleteSnapshot(name, randomFrom(snapshots).snapshotId().getName()).get(); - } + client().admin().cluster().prepareCleanupRepository(name).get(); BlobStoreTestUtil.assertRepoConsistency(internalCluster(), name); }); } else { diff --git a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index dba92dc09c9da..dfbedb874d506 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -488,11 +488,7 @@ public void testSnapshotWithStuckNode() throws Exception { // TODO: Replace this by repository cleanup endpoint call once that's available logger.info("--> Go through a loop of creating and deleting a snapshot to trigger repository cleanup"); - client().admin().cluster().prepareCreateSnapshot("test-repo", "test-snap-tmp") - .setWaitForCompletion(true) - .setIndices("test-idx") - .get(); - client().admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap-tmp").get(); + client().admin().cluster().prepareCleanupRepository("test-repo").get(); // Subtract four files that will remain in the repository: // (1) index-(N+1) From 80624cd8f2968ea01e72475cbb37abd5e8973fdc Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 26 Jun 2019 05:54:03 +0200 Subject: [PATCH 03/66] just ack for now --- .../cleanup/CleanupRepositoryAction.java | 11 ++-- .../CleanupRepositoryRequestBuilder.java | 5 +- .../cleanup/CleanupRepositoryResponse.java | 60 ------------------- .../TransportCleanupRepositoryAction.java | 16 ++--- .../cluster/SnapshotDeletionsInProgress.java | 4 -- .../blobstore/BlobStoreRepository.java | 5 +- 6 files changed, 20 insertions(+), 81 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java index fa711471f3260..ebbe8d2b6963b 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java @@ -19,9 +19,10 @@ package org.elasticsearch.action.admin.cluster.repositories.cleanup; import org.elasticsearch.action.Action; +import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.common.io.stream.Writeable; -public final class CleanupRepositoryAction extends Action { +public final class CleanupRepositoryAction extends Action { public static final CleanupRepositoryAction INSTANCE = new CleanupRepositoryAction(); public static final String NAME = "cluster:admin/repository/cleanup"; @@ -31,12 +32,12 @@ private CleanupRepositoryAction() { } @Override - public CleanupRepositoryResponse newResponse() { - return new CleanupRepositoryResponse(0L, 0L); + public AcknowledgedResponse newResponse() { + throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); } @Override - public Writeable.Reader getResponseReader() { - return CleanupRepositoryResponse::new; + public Writeable.Reader getResponseReader() { + return AcknowledgedResponse::new; } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java index 002c7c0aa05f2..c2ede1b6a4519 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -19,14 +19,15 @@ package org.elasticsearch.action.admin.cluster.repositories.cleanup; import org.elasticsearch.action.Action; +import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.MasterNodeReadOperationRequestBuilder; import org.elasticsearch.client.ElasticsearchClient; public class CleanupRepositoryRequestBuilder extends MasterNodeReadOperationRequestBuilder { - public CleanupRepositoryRequestBuilder(ElasticsearchClient client, Action action, + public CleanupRepositoryRequestBuilder(ElasticsearchClient client, Action action, String repository) { super(client, action, new CleanupRepositoryRequest(repository)); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java deleted file mode 100644 index 3bb3a0efd55cb..0000000000000 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.action.admin.cluster.repositories.cleanup; - -import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.xcontent.ToXContentObject; -import org.elasticsearch.common.xcontent.XContentBuilder; - -import java.io.IOException; - -public final class CleanupRepositoryResponse extends ActionResponse implements ToXContentObject { - - private final long bytes; - - private final long blobs; - - public CleanupRepositoryResponse(long bytes, long blobs) { - this.bytes = bytes; - this.blobs = blobs; - } - - public CleanupRepositoryResponse(StreamInput in) throws IOException { - bytes = in.readLong(); - blobs = in.readLong(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeLong(bytes); - out.writeLong(blobs); - } - - @Override - public void readFrom(StreamInput in) { - throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return builder.startObject().field("bytes", bytes).field("blobs", blobs).endObject(); - } -} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index bfa66c068f64e..e60a4258ade67 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -20,6 +20,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.TransportMasterNodeReadAction; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; @@ -37,7 +38,7 @@ import java.io.IOException; public final class TransportCleanupRepositoryAction extends TransportMasterNodeReadAction { + AcknowledgedResponse> { private final RepositoriesService repositoriesService; @@ -56,21 +57,22 @@ public TransportCleanupRepositoryAction(TransportService transportService, Clust } @Override - protected CleanupRepositoryResponse newResponse() { - return new CleanupRepositoryResponse(0L, 0L); + protected AcknowledgedResponse newResponse() { + return new AcknowledgedResponse(false); } @Override - protected CleanupRepositoryResponse read(StreamInput in) throws IOException { - return new CleanupRepositoryResponse(in); + protected AcknowledgedResponse read(StreamInput in) throws IOException { + return new AcknowledgedResponse(in); } @Override protected void masterOperation(CleanupRepositoryRequest request, ClusterState state, - ActionListener listener) { + ActionListener listener) { Repository repository = repositoriesService.repository(request.repository()); if (repository instanceof BlobStoreRepository) { - ((BlobStoreRepository) repository).cleanup(repository.getRepositoryData().getGenId(), listener); + ((BlobStoreRepository) repository).cleanup(repository.getRepositoryData().getGenId(), + ActionListener.map(listener, AcknowledgedResponse::new)); } else { throw new IllegalArgumentException("Repository [" + request.repository()+ "] is not a blob store repository"); } diff --git a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java index 0134b798c72fd..8e702fbdceea8 100644 --- a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java @@ -44,10 +44,6 @@ public class SnapshotDeletionsInProgress extends AbstractNamedDiffable i // the list of snapshot deletion request entries private final List entries; - public SnapshotDeletionsInProgress() { - this(Collections.emptyList()); - } - private SnapshotDeletionsInProgress(List entries) { this.entries = Collections.unmodifiableList(entries); } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 91cc5c4057e55..5bdd6a89f27c4 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -32,7 +32,6 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRunnable; -import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.support.GroupedActionListener; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaData; @@ -428,12 +427,12 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action } } - public void cleanup(long repositoryStateId, ActionListener listener) { + public void cleanup(long repositoryStateId, ActionListener listener) { ActionListener.completeWith(listener, () -> { final Map foundIndices = blobStore().blobContainer(indicesPath()).children(); final RepositoryData repositoryData = repositoryData(repositoryStateId); cleanupStaleIndices(foundIndices, repositoryData.getIndices()); - return new CleanupRepositoryResponse(0L, 0L); + return true; }); } From 8d22cd57ef2c2b9fd9565a66ebb4cbdc089a5f47 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 26 Jun 2019 05:55:59 +0200 Subject: [PATCH 04/66] just ack for now --- .../java/org/elasticsearch/client/ClusterAdminClient.java | 5 ++--- .../org/elasticsearch/client/support/AbstractClient.java | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java b/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java index fdee39fdb1f93..a5f03dbb0e175 100644 --- a/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java +++ b/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java @@ -51,7 +51,6 @@ import org.elasticsearch.action.admin.cluster.node.usage.NodesUsageResponse; import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequestBuilder; -import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequestBuilder; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesRequest; @@ -464,12 +463,12 @@ public interface ClusterAdminClient extends ElasticsearchClient { /** * Cleans up repository. */ - ActionFuture cleanupRepository(CleanupRepositoryRequest repository); + ActionFuture cleanupRepository(CleanupRepositoryRequest repository); /** * Cleans up repository. */ - void cleanupRepository(CleanupRepositoryRequest repository, ActionListener listener); + void cleanupRepository(CleanupRepositoryRequest repository, ActionListener listener); /** * Verifies a repository. diff --git a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java index dfdda9f76088a..df5100d03c827 100644 --- a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java +++ b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java @@ -67,7 +67,6 @@ import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequestBuilder; -import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequestBuilder; @@ -1014,12 +1013,12 @@ public CleanupRepositoryRequestBuilder prepareCleanupRepository(String repositor } @Override - public ActionFuture cleanupRepository(CleanupRepositoryRequest request) { + public ActionFuture cleanupRepository(CleanupRepositoryRequest request) { return execute(CleanupRepositoryAction.INSTANCE, request); } @Override - public void cleanupRepository(CleanupRepositoryRequest request, ActionListener listener) { + public void cleanupRepository(CleanupRepositoryRequest request, ActionListener listener) { execute(CleanupRepositoryAction.INSTANCE, request, listener); } From 6e14c27723b066942b893bba17b4b07a545b20fd Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 26 Jun 2019 06:05:48 +0200 Subject: [PATCH 05/66] just ack for now --- .../TransportCleanupRepositoryAction.java | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index e60a4258ade67..523f354ae9070 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -29,9 +29,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.repositories.RepositoriesService; -import org.elasticsearch.repositories.Repository; -import org.elasticsearch.repositories.blobstore.BlobStoreRepository; +import org.elasticsearch.snapshots.SnapshotsService; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -40,7 +38,7 @@ public final class TransportCleanupRepositoryAction extends TransportMasterNodeReadAction { - private final RepositoriesService repositoriesService; + private final SnapshotsService snapshotsService; @Override protected String executor() { @@ -49,11 +47,11 @@ protected String executor() { @Inject public TransportCleanupRepositoryAction(TransportService transportService, ClusterService clusterService, - RepositoriesService repositoriesService, ThreadPool threadPool, ActionFilters actionFilters, + SnapshotsService snapshotsService, ThreadPool threadPool, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) { super(CleanupRepositoryAction.NAME, transportService, clusterService, threadPool, actionFilters, CleanupRepositoryRequest::new, indexNameExpressionResolver); - this.repositoriesService = repositoriesService; + this.snapshotsService = snapshotsService; } @Override @@ -69,13 +67,8 @@ protected AcknowledgedResponse read(StreamInput in) throws IOException { @Override protected void masterOperation(CleanupRepositoryRequest request, ClusterState state, ActionListener listener) { - Repository repository = repositoriesService.repository(request.repository()); - if (repository instanceof BlobStoreRepository) { - ((BlobStoreRepository) repository).cleanup(repository.getRepositoryData().getGenId(), - ActionListener.map(listener, AcknowledgedResponse::new)); - } else { - throw new IllegalArgumentException("Repository [" + request.repository()+ "] is not a blob store repository"); - } + snapshotsService.deleteSnapshot(request.repository(), null, + ActionListener.map(listener, v -> new AcknowledgedResponse(true)), false); } @Override From 059eca9ad514d4fd4c9dee6374eb16aadc3e3a29 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 26 Jun 2019 07:05:30 +0200 Subject: [PATCH 06/66] bck --- .../main/java/org/elasticsearch/snapshots/SnapshotsService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 2783d635c90a4..0f9cf22b64808 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -1093,7 +1093,7 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS * @param snapshotName snapshotName * @param listener listener */ - public void deleteSnapshot(final String repositoryName, final String snapshotName, final ActionListener listener, + public void deleteSnapshot(final String repositoryName, @Nullable String snapshotName, final ActionListener listener, final boolean immediatePriority) { // First, look for the snapshot in the repository final Repository repository = repositoriesService.repository(repositoryName); From bfd7e63a2faf4a7ccbf1c83dd9b8fbf9cf51be72 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 26 Jun 2019 17:43:39 +0200 Subject: [PATCH 07/66] bck --- .../cluster/SnapshotDeletionsInProgress.java | 11 +- .../repositories/RepositoriesService.java | 2 +- .../snapshots/SnapshotsService.java | 148 +++++++++++++++++- 3 files changed, 154 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java index 8e702fbdceea8..c01f2d51e1c67 100644 --- a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java @@ -21,6 +21,7 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterState.Custom; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -165,7 +166,7 @@ public String toString() { * A class representing a snapshot deletion request entry in the cluster state. */ public static final class Entry implements Writeable { - private final Snapshot snapshot; + @Nullable private final Snapshot snapshot; private final long startTime; private final long repositoryStateId; @@ -211,7 +212,7 @@ public boolean equals(Object o) { return false; } Entry that = (Entry) o; - return snapshot.equals(that.snapshot) + return Objects.equals(snapshot, that.snapshot) && startTime == that.startTime && repositoryStateId == that.repositoryStateId; } @@ -223,7 +224,11 @@ public int hashCode() { @Override public void writeTo(StreamOutput out) throws IOException { - snapshot.writeTo(out); + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + out.writeOptionalWriteable(snapshot); + } else { + snapshot.writeTo(out); + } out.writeVLong(startTime); out.writeLong(repositoryStateId); } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 88f1051ca2769..5563d3ddeaf00 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -434,7 +434,7 @@ private static void validate(final String repositoryName) { } - private void ensureRepositoryNotInUse(ClusterState clusterState, String repository) { + private static void ensureRepositoryNotInUse(ClusterState clusterState, String repository) { if (SnapshotsService.isRepositoryInUse(clusterState, repository) || RestoreService.isRepositoryInUse(clusterState, repository)) { throw new IllegalStateException("trying to modify or unregister repository that is currently used "); } diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 0f9cf22b64808..27d10bd673e45 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -1110,20 +1110,162 @@ public void deleteSnapshot(final String repositoryName, @Nullable String snapsho // if nothing found by the same name, then look in the cluster state for current in progress snapshots long repoGenId = repositoryData.getGenId(); if (matchedEntry.isPresent() == false) { - Optional matchedInProgress = currentSnapshots(repositoryName, Collections.emptyList()).stream() + final List currentInProgress = currentSnapshots(repositoryName, Collections.emptyList()); + Optional matchedInProgress = currentInProgress.stream() .filter(s -> s.snapshot().getSnapshotId().getName().equals(snapshotName)).findFirst(); if (matchedInProgress.isPresent()) { matchedEntry = matchedInProgress.map(s -> s.snapshot().getSnapshotId()); // Derive repository generation if a snapshot is in progress because it will increment the generation when it finishes repoGenId = matchedInProgress.get().getRepositoryStateId() + 1L; + } else if (currentInProgress.isEmpty() == false) { + repoGenId = currentInProgress.get(0).getRepositoryStateId() + 1L; } } - if (matchedEntry.isPresent() == false) { + if (snapshotName != null && matchedEntry.isPresent() == false) { throw new SnapshotMissingException(repositoryName, snapshotName); } - deleteSnapshot(new Snapshot(repositoryName, matchedEntry.get()), listener, repoGenId, immediatePriority); + if (matchedEntry.isPresent()) { + deleteSnapshot(new Snapshot(repositoryName, matchedEntry.get()), listener, repoGenId, immediatePriority); + } else { + cleanupRepo(repositoryName, listener, repoGenId, immediatePriority); + } } + private void cleanupRepo(String repository, final ActionListener listener, final long repositoryStateId, + final boolean immediatePriority) { + // TODO: this whole method ... + Priority priority = immediatePriority ? Priority.IMMEDIATE : Priority.NORMAL; + clusterService.submitStateUpdateTask("delete snapshot", new ClusterStateUpdateTask(priority) { + + boolean waitForSnapshot = false; + + @Override + public ClusterState execute(ClusterState currentState) { + SnapshotDeletionsInProgress deletionsInProgress = currentState.custom(SnapshotDeletionsInProgress.TYPE); + if (deletionsInProgress != null && deletionsInProgress.hasDeletionsInProgress()) { + throw new IllegalStateException("Cannot clean up - a snapshot is currently being deleted"); + } + ClusterState.Builder clusterStateBuilder = ClusterState.builder(currentState); + SnapshotsInProgress snapshots = currentState.custom(SnapshotsInProgress.TYPE); + SnapshotsInProgress.Entry snapshotEntry = snapshots != null ? snapshots.snapshot(snapshot) : null; + if (snapshotEntry == null) { + // This snapshot is not running - delete + if (snapshots != null && !snapshots.entries().isEmpty()) { + // However other snapshots are running - cannot continue + throw new IllegalStateException("Cannot clean up - a snapshot is currently running"); + } + // add the snapshot deletion to the cluster state + SnapshotDeletionsInProgress.Entry entry = new SnapshotDeletionsInProgress.Entry( + null, + System.currentTimeMillis(), + repositoryStateId + ); + if (deletionsInProgress != null) { + deletionsInProgress = deletionsInProgress.withAddedEntry(entry); + } else { + deletionsInProgress = SnapshotDeletionsInProgress.newInstance(entry); + } + clusterStateBuilder.putCustom(SnapshotDeletionsInProgress.TYPE, deletionsInProgress); + } else { + // This snapshot is currently running - stopping shards first + waitForSnapshot = true; + + final ImmutableOpenMap shards; + + final State state = snapshotEntry.state(); + final String failure; + if (state == State.INIT) { + // snapshot is still initializing, mark it as aborted + shards = snapshotEntry.shards(); + assert shards.isEmpty(); + failure = "Snapshot was aborted during initialization"; + } else if (state == State.STARTED) { + // snapshot is started - mark every non completed shard as aborted + final ImmutableOpenMap.Builder shardsBuilder = ImmutableOpenMap.builder(); + for (ObjectObjectCursor shardEntry : snapshotEntry.shards()) { + ShardSnapshotStatus status = shardEntry.value; + if (status.state().completed() == false) { + status = new ShardSnapshotStatus(status.nodeId(), ShardState.ABORTED, "aborted by snapshot deletion"); + } + shardsBuilder.put(shardEntry.key, status); + } + shards = shardsBuilder.build(); + failure = "Snapshot was aborted by deletion"; + } else { + boolean hasUncompletedShards = false; + // Cleanup in case a node gone missing and snapshot wasn't updated for some reason + for (ObjectCursor shardStatus : snapshotEntry.shards().values()) { + // Check if we still have shard running on existing nodes + if (shardStatus.value.state().completed() == false && shardStatus.value.nodeId() != null + && currentState.nodes().get(shardStatus.value.nodeId()) != null) { + hasUncompletedShards = true; + break; + } + } + if (hasUncompletedShards) { + // snapshot is being finalized - wait for shards to complete finalization process + logger.debug("trying to delete completed snapshot - should wait for shards to finalize on all nodes"); + return currentState; + } else { + // no shards to wait for but a node is gone - this is the only case + // where we force to finish the snapshot + logger.debug("trying to delete completed snapshot with no finalizing shards - can delete immediately"); + shards = snapshotEntry.shards(); + } + failure = snapshotEntry.failure(); + } + SnapshotsInProgress.Entry newSnapshot = new SnapshotsInProgress.Entry(snapshotEntry, State.ABORTED, shards, failure); + clusterStateBuilder.putCustom(SnapshotsInProgress.TYPE, new SnapshotsInProgress(newSnapshot)); + } + return clusterStateBuilder.build(); + } + + @Override + public void onFailure(String source, Exception e) { + listener.onFailure(e); + } + + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + if (waitForSnapshot) { + logger.trace("adding snapshot completion listener to wait for deleted snapshot to finish"); + addListener(snapshot, ActionListener.wrap( + snapshotInfo -> { + logger.debug("deleted snapshot completed - deleting files"); + threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(() -> { + try { + deleteSnapshot(snapshot.getRepository(), snapshot.getSnapshotId().getName(), listener, true); + } catch (Exception ex) { + logger.warn(() -> new ParameterizedMessage("[{}] failed to delete snapshot", snapshot), ex); + } + } + ); + }, + e -> { + logger.warn("deleted snapshot failed - deleting files", e); + threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(() -> { + try { + deleteSnapshot(snapshot.getRepository(), snapshot.getSnapshotId().getName(), listener, true); + } catch (SnapshotMissingException smex) { + logger.info(() -> new ParameterizedMessage( + "Tried deleting in-progress snapshot [{}], but it could not be found after failing to abort.", + smex.getSnapshotName()), e); + listener.onFailure(new SnapshotException(snapshot, + "Tried deleting in-progress snapshot [" + smex.getSnapshotName() + "], but it " + + "could not be found after failing to abort.", smex)); + } + }); + } + )); + } else { + logger.debug("deleted snapshot is not running - deleting files"); + deleteSnapshotFromRepository(snapshot, listener, repositoryStateId); + } + } + }); + } + + /** * Deletes snapshot from repository. *

From d2952d47b2eae07e73564ec93b2a1d41af7154af Mon Sep 17 00:00:00 2001 From: Armin Date: Thu, 27 Jun 2019 14:04:16 +0200 Subject: [PATCH 08/66] bck --- .../TransportCleanupRepositoryAction.java | 3 +- .../cluster/SnapshotDeletionsInProgress.java | 12 +- .../blobstore/BlobStoreRepository.java | 4 +- .../org/elasticsearch/snapshots/Snapshot.java | 20 ++- .../snapshots/SnapshotsService.java | 130 ++++-------------- 5 files changed, 49 insertions(+), 120 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 523f354ae9070..03ae413a279f4 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.snapshots.SnapshotsService; +import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -65,7 +66,7 @@ protected AcknowledgedResponse read(StreamInput in) throws IOException { } @Override - protected void masterOperation(CleanupRepositoryRequest request, ClusterState state, + protected void masterOperation(Task task, CleanupRepositoryRequest request, ClusterState state, ActionListener listener) { snapshotsService.deleteSnapshot(request.repository(), null, ActionListener.map(listener, v -> new AcknowledgedResponse(true)), false); diff --git a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java index c01f2d51e1c67..779f916872d05 100644 --- a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java @@ -21,7 +21,6 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.ClusterState.Custom; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -137,6 +136,9 @@ public Version getMinimalSupportedVersion() { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startArray(TYPE); for (Entry entry : entries) { + if (entry.snapshot.getSnapshotId() == null) { + continue; + } builder.startObject(); { builder.field("repository", entry.snapshot.getRepository()); @@ -166,7 +168,7 @@ public String toString() { * A class representing a snapshot deletion request entry in the cluster state. */ public static final class Entry implements Writeable { - @Nullable private final Snapshot snapshot; + private final Snapshot snapshot; private final long startTime; private final long repositoryStateId; @@ -224,11 +226,7 @@ public int hashCode() { @Override public void writeTo(StreamOutput out) throws IOException { - if (out.getVersion().onOrAfter(Version.V_8_0_0)) { - out.writeOptionalWriteable(snapshot); - } else { - snapshot.writeTo(out); - } + snapshot.writeTo(out); out.writeVLong(startTime); out.writeLong(repositoryStateId); } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 5bdd6a89f27c4..91ceab3b92e3f 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -427,12 +427,12 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action } } - public void cleanup(long repositoryStateId, ActionListener listener) { + public void cleanup(long repositoryStateId, ActionListener listener) { ActionListener.completeWith(listener, () -> { final Map foundIndices = blobStore().blobContainer(indicesPath()).children(); final RepositoryData repositoryData = repositoryData(repositoryStateId); cleanupStaleIndices(foundIndices, repositoryData.getIndices()); - return true; + return null; }); } diff --git a/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java b/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java index 2847af386b2e1..e66724912888f 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java +++ b/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java @@ -19,6 +19,8 @@ package org.elasticsearch.snapshots; +import org.elasticsearch.Version; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -38,9 +40,9 @@ public final class Snapshot implements Writeable { /** * Constructs a snapshot. */ - public Snapshot(final String repository, final SnapshotId snapshotId) { + public Snapshot(String repository, @Nullable SnapshotId snapshotId) { this.repository = Objects.requireNonNull(repository); - this.snapshotId = Objects.requireNonNull(snapshotId); + this.snapshotId = snapshotId; this.hashCode = computeHashCode(); } @@ -49,7 +51,11 @@ public Snapshot(final String repository, final SnapshotId snapshotId) { */ public Snapshot(final StreamInput in) throws IOException { repository = in.readString(); - snapshotId = new SnapshotId(in); + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + snapshotId = in.readOptionalWriteable(SnapshotId::new); + } else { + snapshotId = new SnapshotId(in); + } hashCode = computeHashCode(); } @@ -81,7 +87,7 @@ public boolean equals(Object o) { return false; } Snapshot that = (Snapshot) o; - return repository.equals(that.repository) && snapshotId.equals(that.snapshotId); + return repository.equals(that.repository) && Objects.equals(snapshotId, that.snapshotId); } @Override @@ -96,7 +102,11 @@ private int computeHashCode() { @Override public void writeTo(final StreamOutput out) throws IOException { out.writeString(repository); - snapshotId.writeTo(out); + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + out.writeOptionalWriteable(snapshotId); + } else { + snapshotId.writeTo(out); + } } } diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 27d10bd673e45..61adfde8fa73d 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -68,6 +68,7 @@ import org.elasticsearch.repositories.Repository; import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.RepositoryMissingException; +import org.elasticsearch.repositories.blobstore.BlobStoreRepository; import org.elasticsearch.threadpool.ThreadPool; import java.io.IOException; @@ -1131,14 +1132,11 @@ public void deleteSnapshot(final String repositoryName, @Nullable String snapsho } } - private void cleanupRepo(String repository, final ActionListener listener, final long repositoryStateId, + private void cleanupRepo(final String repositoryName, final ActionListener listener, final long repositoryStateId, final boolean immediatePriority) { // TODO: this whole method ... Priority priority = immediatePriority ? Priority.IMMEDIATE : Priority.NORMAL; clusterService.submitStateUpdateTask("delete snapshot", new ClusterStateUpdateTask(priority) { - - boolean waitForSnapshot = false; - @Override public ClusterState execute(ClusterState currentState) { SnapshotDeletionsInProgress deletionsInProgress = currentState.custom(SnapshotDeletionsInProgress.TYPE); @@ -1147,76 +1145,22 @@ public ClusterState execute(ClusterState currentState) { } ClusterState.Builder clusterStateBuilder = ClusterState.builder(currentState); SnapshotsInProgress snapshots = currentState.custom(SnapshotsInProgress.TYPE); - SnapshotsInProgress.Entry snapshotEntry = snapshots != null ? snapshots.snapshot(snapshot) : null; - if (snapshotEntry == null) { - // This snapshot is not running - delete - if (snapshots != null && !snapshots.entries().isEmpty()) { - // However other snapshots are running - cannot continue - throw new IllegalStateException("Cannot clean up - a snapshot is currently running"); - } - // add the snapshot deletion to the cluster state - SnapshotDeletionsInProgress.Entry entry = new SnapshotDeletionsInProgress.Entry( - null, - System.currentTimeMillis(), - repositoryStateId - ); - if (deletionsInProgress != null) { - deletionsInProgress = deletionsInProgress.withAddedEntry(entry); - } else { - deletionsInProgress = SnapshotDeletionsInProgress.newInstance(entry); - } - clusterStateBuilder.putCustom(SnapshotDeletionsInProgress.TYPE, deletionsInProgress); + if (snapshots != null && !snapshots.entries().isEmpty()) { + // However other snapshots are running - cannot continue + throw new IllegalStateException("Cannot clean up - a snapshot is currently running"); + } + // add the snapshot deletion to the cluster state + SnapshotDeletionsInProgress.Entry entry = new SnapshotDeletionsInProgress.Entry( + new Snapshot(repositoryName, null), + System.currentTimeMillis(), + repositoryStateId + ); + if (deletionsInProgress != null) { + deletionsInProgress = deletionsInProgress.withAddedEntry(entry); } else { - // This snapshot is currently running - stopping shards first - waitForSnapshot = true; - - final ImmutableOpenMap shards; - - final State state = snapshotEntry.state(); - final String failure; - if (state == State.INIT) { - // snapshot is still initializing, mark it as aborted - shards = snapshotEntry.shards(); - assert shards.isEmpty(); - failure = "Snapshot was aborted during initialization"; - } else if (state == State.STARTED) { - // snapshot is started - mark every non completed shard as aborted - final ImmutableOpenMap.Builder shardsBuilder = ImmutableOpenMap.builder(); - for (ObjectObjectCursor shardEntry : snapshotEntry.shards()) { - ShardSnapshotStatus status = shardEntry.value; - if (status.state().completed() == false) { - status = new ShardSnapshotStatus(status.nodeId(), ShardState.ABORTED, "aborted by snapshot deletion"); - } - shardsBuilder.put(shardEntry.key, status); - } - shards = shardsBuilder.build(); - failure = "Snapshot was aborted by deletion"; - } else { - boolean hasUncompletedShards = false; - // Cleanup in case a node gone missing and snapshot wasn't updated for some reason - for (ObjectCursor shardStatus : snapshotEntry.shards().values()) { - // Check if we still have shard running on existing nodes - if (shardStatus.value.state().completed() == false && shardStatus.value.nodeId() != null - && currentState.nodes().get(shardStatus.value.nodeId()) != null) { - hasUncompletedShards = true; - break; - } - } - if (hasUncompletedShards) { - // snapshot is being finalized - wait for shards to complete finalization process - logger.debug("trying to delete completed snapshot - should wait for shards to finalize on all nodes"); - return currentState; - } else { - // no shards to wait for but a node is gone - this is the only case - // where we force to finish the snapshot - logger.debug("trying to delete completed snapshot with no finalizing shards - can delete immediately"); - shards = snapshotEntry.shards(); - } - failure = snapshotEntry.failure(); - } - SnapshotsInProgress.Entry newSnapshot = new SnapshotsInProgress.Entry(snapshotEntry, State.ABORTED, shards, failure); - clusterStateBuilder.putCustom(SnapshotsInProgress.TYPE, new SnapshotsInProgress(newSnapshot)); + deletionsInProgress = SnapshotDeletionsInProgress.newInstance(entry); } + clusterStateBuilder.putCustom(SnapshotDeletionsInProgress.TYPE, deletionsInProgress); return clusterStateBuilder.build(); } @@ -1227,40 +1171,16 @@ public void onFailure(String source, Exception e) { @Override public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { - if (waitForSnapshot) { - logger.trace("adding snapshot completion listener to wait for deleted snapshot to finish"); - addListener(snapshot, ActionListener.wrap( - snapshotInfo -> { - logger.debug("deleted snapshot completed - deleting files"); - threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(() -> { - try { - deleteSnapshot(snapshot.getRepository(), snapshot.getSnapshotId().getName(), listener, true); - } catch (Exception ex) { - logger.warn(() -> new ParameterizedMessage("[{}] failed to delete snapshot", snapshot), ex); - } - } - ); - }, - e -> { - logger.warn("deleted snapshot failed - deleting files", e); - threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(() -> { - try { - deleteSnapshot(snapshot.getRepository(), snapshot.getSnapshotId().getName(), listener, true); - } catch (SnapshotMissingException smex) { - logger.info(() -> new ParameterizedMessage( - "Tried deleting in-progress snapshot [{}], but it could not be found after failing to abort.", - smex.getSnapshotName()), e); - listener.onFailure(new SnapshotException(snapshot, - "Tried deleting in-progress snapshot [" + smex.getSnapshotName() + "], but it " + - "could not be found after failing to abort.", smex)); - } - }); + threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(new ActionRunnable<>(listener) { + @Override + protected void doRun() { + Repository repository = repositoriesService.repository(repositoryName); + if (repository instanceof BlobStoreRepository == false) { + throw new IllegalArgumentException("Repository [" + repositoryName + "] is not a blob store repository"); } - )); - } else { - logger.debug("deleted snapshot is not running - deleting files"); - deleteSnapshotFromRepository(snapshot, listener, repositoryStateId); - } + ((BlobStoreRepository) repository).cleanup(repositoryStateId, listener); + } + }); } }); } From c54a075e847bae2009fcbc0ec40d753dc749f8c5 Mon Sep 17 00:00:00 2001 From: Armin Date: Fri, 28 Jun 2019 13:30:42 +0200 Subject: [PATCH 09/66] Fix functionality --- .../org/elasticsearch/snapshots/SnapshotsService.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 61adfde8fa73d..325fb9d8d9903 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -1132,9 +1132,7 @@ public void deleteSnapshot(final String repositoryName, @Nullable String snapsho } } - private void cleanupRepo(final String repositoryName, final ActionListener listener, final long repositoryStateId, - final boolean immediatePriority) { - // TODO: this whole method ... + private void cleanupRepo(String repositoryName, ActionListener listener, long repositoryStateId, boolean immediatePriority) { Priority priority = immediatePriority ? Priority.IMMEDIATE : Priority.NORMAL; clusterService.submitStateUpdateTask("delete snapshot", new ClusterStateUpdateTask(priority) { @Override @@ -1166,7 +1164,7 @@ public ClusterState execute(ClusterState currentState) { @Override public void onFailure(String source, Exception e) { - listener.onFailure(e); + removeSnapshotDeletionFromClusterState(null, e, listener); } @Override @@ -1178,7 +1176,9 @@ protected void doRun() { if (repository instanceof BlobStoreRepository == false) { throw new IllegalArgumentException("Repository [" + repositoryName + "] is not a blob store repository"); } - ((BlobStoreRepository) repository).cleanup(repositoryStateId, listener); + ((BlobStoreRepository) repository).cleanup(repositoryStateId, ActionListener.wrap( + v -> removeSnapshotDeletionFromClusterState(null, null, listener), + e -> removeSnapshotDeletionFromClusterState(null, e, listener))); } }); } From 71446c56d95939b5531d041187cb8c776d4c2a03 Mon Sep 17 00:00:00 2001 From: Armin Date: Fri, 28 Jun 2019 14:24:54 +0200 Subject: [PATCH 10/66] add rest action --- .../elasticsearch/action/ActionModule.java | 2 + .../cleanup/CleanupRepositoryRequest.java | 4 +- .../org/elasticsearch/client/Requests.java | 11 ++++ .../cluster/RestCleanupRepositoryAction.java | 56 +++++++++++++++++++ 4 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index f43aa71eddbb3..609027de467b0 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -222,6 +222,7 @@ import org.elasticsearch.rest.action.RestMainAction; import org.elasticsearch.rest.action.admin.cluster.RestAddVotingConfigExclusionAction; import org.elasticsearch.rest.action.admin.cluster.RestCancelTasksAction; +import org.elasticsearch.rest.action.admin.cluster.RestCleanupRepositoryAction; import org.elasticsearch.rest.action.admin.cluster.RestClearVotingConfigExclusionsAction; import org.elasticsearch.rest.action.admin.cluster.RestClusterAllocationExplainAction; import org.elasticsearch.rest.action.admin.cluster.RestClusterGetSettingsAction; @@ -564,6 +565,7 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestPutRepositoryAction(settings, restController)); registerHandler.accept(new RestGetRepositoriesAction(settings, restController, settingsFilter)); registerHandler.accept(new RestDeleteRepositoryAction(settings, restController)); + registerHandler.accept(new RestCleanupRepositoryAction(settings, restController)); registerHandler.accept(new RestVerifyRepositoryAction(settings, restController)); registerHandler.accept(new RestGetSnapshotsAction(settings, restController)); registerHandler.accept(new RestCreateSnapshotAction(settings, restController)); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java index c9c6e19ca7ebd..12982976b0959 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java @@ -19,7 +19,7 @@ package org.elasticsearch.action.admin.cluster.repositories.cleanup; import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.action.support.master.MasterNodeReadRequest; +import org.elasticsearch.action.support.master.AcknowledgedRequest; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -27,7 +27,7 @@ import static org.elasticsearch.action.ValidateActions.addValidationError; -public class CleanupRepositoryRequest extends MasterNodeReadRequest { +public class CleanupRepositoryRequest extends AcknowledgedRequest { private String repository; diff --git a/server/src/main/java/org/elasticsearch/client/Requests.java b/server/src/main/java/org/elasticsearch/client/Requests.java index a3eb23eebfe20..fa7bc73c8b9fc 100644 --- a/server/src/main/java/org/elasticsearch/client/Requests.java +++ b/server/src/main/java/org/elasticsearch/client/Requests.java @@ -26,6 +26,7 @@ import org.elasticsearch.action.admin.cluster.node.tasks.get.GetTaskRequest; import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksRequest; import org.elasticsearch.action.admin.cluster.node.usage.NodesUsageRequest; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesRequest; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest; @@ -460,6 +461,16 @@ public static DeleteRepositoryRequest deleteRepositoryRequest(String name) { return new DeleteRepositoryRequest(name); } + /** + * Cleanup repository + * + * @param name repository name + * @return cleanup repository request + */ + public static CleanupRepositoryRequest cleanupRepositoryRequest(String name) { + return new CleanupRepositoryRequest(name); + } + /** * Verifies snapshot repository * diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java new file mode 100644 index 0000000000000..fa2e06594e9e3 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java @@ -0,0 +1,56 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.rest.action.admin.cluster; + +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.rest.BaseRestHandler; +import org.elasticsearch.rest.RestController; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.action.RestToXContentListener; + +import java.io.IOException; + +import static org.elasticsearch.client.Requests.cleanupRepositoryRequest; +import static org.elasticsearch.rest.RestRequest.Method.POST; + +/** + * Cleans up a repository + */ +public class RestCleanupRepositoryAction extends BaseRestHandler { + public RestCleanupRepositoryAction(Settings settings, RestController controller) { + super(settings); + controller.registerHandler(POST, "/_snapshot/{repository}/cleanup", this); + } + + @Override + public String getName() { + return "cleanup_action"; + } + + @Override + public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + CleanupRepositoryRequest cleanupRepositoryRequest = cleanupRepositoryRequest(request.param("repository")); + cleanupRepositoryRequest.timeout(request.paramAsTime("timeout", cleanupRepositoryRequest.timeout())); + cleanupRepositoryRequest.masterNodeTimeout(request.paramAsTime("master_timeout", cleanupRepositoryRequest.masterNodeTimeout())); + return channel -> client.admin().cluster().cleanupRepository(cleanupRepositoryRequest, new RestToXContentListener<>(channel)); + } +} From 48da5d025e829fd1c5bf2d634b7340e0450897c6 Mon Sep 17 00:00:00 2001 From: Armin Date: Fri, 28 Jun 2019 14:28:29 +0200 Subject: [PATCH 11/66] fix compilation --- .../cleanup/TransportCleanupRepositoryAction.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 03ae413a279f4..f7d144094f69c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -21,7 +21,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.action.support.master.TransportMasterNodeReadAction; +import org.elasticsearch.action.support.master.TransportMasterNodeAction; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; @@ -36,7 +36,7 @@ import java.io.IOException; -public final class TransportCleanupRepositoryAction extends TransportMasterNodeReadAction { private final SnapshotsService snapshotsService; From 6f2e702273201bd4fb5cee7b81670e58e5735abd Mon Sep 17 00:00:00 2001 From: Armin Date: Fri, 28 Jun 2019 14:29:17 +0200 Subject: [PATCH 12/66] fix compilation --- .../repositories/cleanup/CleanupRepositoryRequestBuilder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java index c2ede1b6a4519..517f8bebce1be 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -20,10 +20,10 @@ import org.elasticsearch.action.Action; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.action.support.master.MasterNodeReadOperationRequestBuilder; +import org.elasticsearch.action.support.master.MasterNodeOperationRequestBuilder; import org.elasticsearch.client.ElasticsearchClient; -public class CleanupRepositoryRequestBuilder extends MasterNodeReadOperationRequestBuilder { From 5a0c826a66c2c181d248361f273cc6ef2dd95359 Mon Sep 17 00:00:00 2001 From: Armin Date: Fri, 28 Jun 2019 14:49:11 +0200 Subject: [PATCH 13/66] nicer formatting --- .../repositories/cleanup/CleanupRepositoryRequestBuilder.java | 4 ++-- .../cleanup/TransportCleanupRepositoryAction.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java index 517f8bebce1be..57ad7475712f5 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -24,8 +24,8 @@ import org.elasticsearch.client.ElasticsearchClient; public class CleanupRepositoryRequestBuilder extends MasterNodeOperationRequestBuilder { + AcknowledgedResponse, + CleanupRepositoryRequestBuilder> { public CleanupRepositoryRequestBuilder(ElasticsearchClient client, Action action, String repository) { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index f7d144094f69c..c039a6d3011eb 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -37,7 +37,7 @@ import java.io.IOException; public final class TransportCleanupRepositoryAction extends TransportMasterNodeAction { + AcknowledgedResponse> { private final SnapshotsService snapshotsService; From 2bbd0f0d773f83fa30d49cc31e7ab7297352148e Mon Sep 17 00:00:00 2001 From: Armin Date: Fri, 28 Jun 2019 16:56:32 +0200 Subject: [PATCH 14/66] bck --- .../repositories/blobstore/BlobStoreRepository.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 91ceab3b92e3f..44ba185713b03 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -427,8 +427,19 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action } } + /** + * Runs all cleanup actions on the repository. + * TODO: Add top level blob cleanup + * TODO: Add shard level cleanups + *

    + *
  • Deleting stale indices {@link #cleanupStaleIndices(Map, Map)}
    • + *
+ * @param repositoryStateId Current repository state id + * @param listener Lister to complete when done + */ public void cleanup(long repositoryStateId, ActionListener listener) { ActionListener.completeWith(listener, () -> { + // TODO: ensure state id final Map foundIndices = blobStore().blobContainer(indicesPath()).children(); final RepositoryData repositoryData = repositoryData(repositoryStateId); cleanupStaleIndices(foundIndices, repositoryData.getIndices()); From b2ebb527cfa7c58fa9b03a6b638426d7a4fc61a8 Mon Sep 17 00:00:00 2001 From: Armin Date: Sun, 30 Jun 2019 20:44:16 +0200 Subject: [PATCH 15/66] merge fixes --- .../cleanup/CleanupRepositoryAction.java | 17 +++-------------- .../CleanupRepositoryRequestBuilder.java | 4 ++-- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java index ebbe8d2b6963b..f5f9515361c34 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java @@ -18,26 +18,15 @@ */ package org.elasticsearch.action.admin.cluster.repositories.cleanup; -import org.elasticsearch.action.Action; +import org.elasticsearch.action.ActionType; import org.elasticsearch.action.support.master.AcknowledgedResponse; -import org.elasticsearch.common.io.stream.Writeable; -public final class CleanupRepositoryAction extends Action { +public final class CleanupRepositoryAction extends ActionType { public static final CleanupRepositoryAction INSTANCE = new CleanupRepositoryAction(); public static final String NAME = "cluster:admin/repository/cleanup"; private CleanupRepositoryAction() { - super(NAME); - } - - @Override - public AcknowledgedResponse newResponse() { - throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); - } - - @Override - public Writeable.Reader getResponseReader() { - return AcknowledgedResponse::new; + super(NAME, AcknowledgedResponse::new); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java index 57ad7475712f5..cb9ecbda7aec9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -18,7 +18,7 @@ */ package org.elasticsearch.action.admin.cluster.repositories.cleanup; -import org.elasticsearch.action.Action; +import org.elasticsearch.action.ActionType; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.MasterNodeOperationRequestBuilder; import org.elasticsearch.client.ElasticsearchClient; @@ -27,7 +27,7 @@ public class CleanupRepositoryRequestBuilder extends MasterNodeOperationRequestB AcknowledgedResponse, CleanupRepositoryRequestBuilder> { - public CleanupRepositoryRequestBuilder(ElasticsearchClient client, Action action, + public CleanupRepositoryRequestBuilder(ElasticsearchClient client, ActionType action, String repository) { super(client, action, new CleanupRepositoryRequest(repository)); } From ba1ad03f250bae303d160a3bac9860cfd0a7a857 Mon Sep 17 00:00:00 2001 From: Armin Date: Sun, 30 Jun 2019 21:19:10 +0200 Subject: [PATCH 16/66] add some documentation --- docs/reference/modules/snapshots.asciidoc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/reference/modules/snapshots.asciidoc b/docs/reference/modules/snapshots.asciidoc index ed52c0958fec0..e20e55ab5fc25 100644 --- a/docs/reference/modules/snapshots.asciidoc +++ b/docs/reference/modules/snapshots.asciidoc @@ -324,6 +324,24 @@ POST /_snapshot/my_unverified_backup/_verify It returns a list of nodes where repository was successfully verified or an error message if verification process failed. +[float] +===== Repository Cleanup +Repositories can over time accumulate data that is not referenced by any existing snapshot. This is a result of the data safety guarantees +the snapshot functionality provides in failure scenarios during snapshot creation and the decentralized nature of the snapshot creation +process. This unreferenced data does in no way negatively impact the performance or safety of a snapshot repository but leads to higher +than necessary storage use. In order to clean up this unreferenced data, users can call the cleanup endpoint for a repository which will +trigger a complete accounting of the repositories contents and subsequent deletion of all unreferenced data that was found. + +[source,js] +----------------------------------- +POST /_snapshot/my_repository/cleanup +----------------------------------- +// CONSOLE +// TEST[continued] + +NOTE: Most of the cleanup actions performed by the repository cleanup functionality are automatically executed when deleting a snapshot. +If you regularly delete snapshots from a repository you likely will get little to no space savings from executing this functionality. + [float] === Snapshot From 266471f1cbef33f43b0c337f09e11a5c569dd6b8 Mon Sep 17 00:00:00 2001 From: Armin Date: Sun, 30 Jun 2019 21:29:05 +0200 Subject: [PATCH 17/66] nicer cleanup logic --- .../snapshots/SnapshotsService.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 325fb9d8d9903..a0a796e7bcf2a 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -1098,6 +1098,10 @@ public void deleteSnapshot(final String repositoryName, @Nullable String snapsho final boolean immediatePriority) { // First, look for the snapshot in the repository final Repository repository = repositoriesService.repository(repositoryName); + if (snapshotName == null && repository instanceof BlobStoreRepository == false) { + listener.onResponse(null); + return; + } final RepositoryData repositoryData = repository.getRepositoryData(); final Optional incompatibleSnapshotId = repositoryData.getIncompatibleSnapshotIds().stream().filter(s -> snapshotName.equals(s.getName())).findFirst(); @@ -1164,7 +1168,7 @@ public ClusterState execute(ClusterState currentState) { @Override public void onFailure(String source, Exception e) { - removeSnapshotDeletionFromClusterState(null, e, listener); + after(e); } @Override @@ -1173,15 +1177,16 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS @Override protected void doRun() { Repository repository = repositoriesService.repository(repositoryName); - if (repository instanceof BlobStoreRepository == false) { - throw new IllegalArgumentException("Repository [" + repositoryName + "] is not a blob store repository"); - } - ((BlobStoreRepository) repository).cleanup(repositoryStateId, ActionListener.wrap( - v -> removeSnapshotDeletionFromClusterState(null, null, listener), - e -> removeSnapshotDeletionFromClusterState(null, e, listener))); + assert repository instanceof BlobStoreRepository; + ((BlobStoreRepository) repository) + .cleanup(repositoryStateId, ActionListener.wrap(v -> after(null), e -> after(e))); } }); } + + private void after(@Nullable Exception e) { + removeSnapshotDeletionFromClusterState(null, e, listener); + } }); } From 28a4b697f5bdf2ef4285175db6828e5187528ca8 Mon Sep 17 00:00:00 2001 From: Armin Date: Mon, 1 Jul 2019 10:43:56 +0200 Subject: [PATCH 18/66] bck --- .../cleanup/CleanupRepositoryResponse.java | 14 ++++ .../TransportCleanupRepositoryAction.java | 1 + .../repositories/RepositoryCleanupResult.java | 74 +++++++++++++++++++ .../snapshots/SnapshotsService.java | 4 +- 4 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java create mode 100644 server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java new file mode 100644 index 0000000000000..406b778e6fa68 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java @@ -0,0 +1,14 @@ +package org.elasticsearch.action.admin.cluster.repositories.cleanup; + +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; + +public class CleanupRepositoryResponse extends ActionResponse implements ToXContentObject { + @Override + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + return null; + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index c039a6d3011eb..9f74af8d6f761 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -36,6 +36,7 @@ import java.io.IOException; +// TODO: add response that includes stats? public final class TransportCleanupRepositoryAction extends TransportMasterNodeAction { diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java new file mode 100644 index 0000000000000..0fdbfe4d08293 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java @@ -0,0 +1,74 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.repositories; + +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.stream.LongStream; + +public final class RepositoryCleanupResult implements Writeable, ToXContentObject { + + private final long bytes; + + private final long blobs; + + private RepositoryCleanupResult(long bytes, long blobs) { + this.bytes = bytes; + this.blobs = blobs; + } + + public static Progress start() { + return new Progress(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeLong(bytes); + out.writeLong(blobs); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return builder.startObject().field("bytes", bytes).field("blobs", blobs).endObject(); + } + + public static final class Progress { + + private long bytesCounter; + + private long blobsCounter; + + public void add(long[] sizes) { + synchronized (this) { + blobsCounter += sizes.length; + bytesCounter += LongStream.of(sizes).sum(); + } + } + + public RepositoryCleanupResult finish() { + synchronized (this) { + return new RepositoryCleanupResult(bytesCounter, blobsCounter); + } + } + } +} diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index a0a796e7bcf2a..133107a75ce17 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -1099,7 +1099,7 @@ public void deleteSnapshot(final String repositoryName, @Nullable String snapsho // First, look for the snapshot in the repository final Repository repository = repositoriesService.repository(repositoryName); if (snapshotName == null && repository instanceof BlobStoreRepository == false) { - listener.onResponse(null); + listener.onFailure(new IllegalArgumentException("Repository [" + repositoryName + "] does not support repository cleanup")); return; } final RepositoryData repositoryData = repository.getRepositoryData(); @@ -1136,7 +1136,7 @@ public void deleteSnapshot(final String repositoryName, @Nullable String snapsho } } - private void cleanupRepo(String repositoryName, ActionListener listener, long repositoryStateId, boolean immediatePriority) { + public void cleanupRepo(String repositoryName, ActionListener listener, long repositoryStateId, boolean immediatePriority) { Priority priority = immediatePriority ? Priority.IMMEDIATE : Priority.NORMAL; clusterService.submitStateUpdateTask("delete snapshot", new ClusterStateUpdateTask(priority) { @Override From a0493dfbfb808d59e13f1cbc853cae96e6930796 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 13:29:06 +0200 Subject: [PATCH 19/66] result response --- .../cleanup/CleanupRepositoryAction.java | 5 +- .../CleanupRepositoryRequestBuilder.java | 5 +- .../cleanup/CleanupRepositoryResponse.java | 47 +++++++++++++++++-- .../TransportCleanupRepositoryAction.java | 17 ++++--- .../client/ClusterAdminClient.java | 5 +- .../client/support/AbstractClient.java | 5 +- .../repositories/RepositoryCleanupResult.java | 6 +++ .../blobstore/BlobStoreRepository.java | 5 +- .../snapshots/SnapshotsService.java | 40 ++++++++-------- 9 files changed, 91 insertions(+), 44 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java index f5f9515361c34..d5b28b4534aca 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java @@ -19,14 +19,13 @@ package org.elasticsearch.action.admin.cluster.repositories.cleanup; import org.elasticsearch.action.ActionType; -import org.elasticsearch.action.support.master.AcknowledgedResponse; -public final class CleanupRepositoryAction extends ActionType { +public final class CleanupRepositoryAction extends ActionType { public static final CleanupRepositoryAction INSTANCE = new CleanupRepositoryAction(); public static final String NAME = "cluster:admin/repository/cleanup"; private CleanupRepositoryAction() { - super(NAME, AcknowledgedResponse::new); + super(NAME, CleanupRepositoryResponse::new); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java index cb9ecbda7aec9..d202877eee941 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -19,15 +19,14 @@ package org.elasticsearch.action.admin.cluster.repositories.cleanup; import org.elasticsearch.action.ActionType; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.MasterNodeOperationRequestBuilder; import org.elasticsearch.client.ElasticsearchClient; public class CleanupRepositoryRequestBuilder extends MasterNodeOperationRequestBuilder { - public CleanupRepositoryRequestBuilder(ElasticsearchClient client, ActionType action, + public CleanupRepositoryRequestBuilder(ElasticsearchClient client, ActionType action, String repository) { super(client, action, new CleanupRepositoryRequest(repository)); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java index 406b778e6fa68..fa581860f3a48 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java @@ -1,14 +1,55 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ package org.elasticsearch.action.admin.cluster.repositories.cleanup; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.repositories.RepositoryCleanupResult; import java.io.IOException; -public class CleanupRepositoryResponse extends ActionResponse implements ToXContentObject { +public final class CleanupRepositoryResponse extends ActionResponse implements ToXContentObject { + + private final RepositoryCleanupResult result; + + public CleanupRepositoryResponse(RepositoryCleanupResult result) { + this.result = result; + } + + public CleanupRepositoryResponse(StreamInput in) throws IOException { + result = new RepositoryCleanupResult(in); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + result.writeTo(out); + } + @Override - public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { - return null; + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject().field("results"); + result.toXContent(builder, params); + builder.endObject(); + return builder; } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 9f74af8d6f761..e1358ece732f9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -20,7 +20,6 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; -import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.action.support.master.TransportMasterNodeAction; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; @@ -29,6 +28,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.repositories.RepositoryCleanupResult; import org.elasticsearch.snapshots.SnapshotsService; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; @@ -38,7 +38,7 @@ // TODO: add response that includes stats? public final class TransportCleanupRepositoryAction extends TransportMasterNodeAction { + CleanupRepositoryResponse> { private final SnapshotsService snapshotsService; @@ -57,20 +57,19 @@ public TransportCleanupRepositoryAction(TransportService transportService, Clust } @Override - protected AcknowledgedResponse newResponse() { - return new AcknowledgedResponse(false); + protected CleanupRepositoryResponse newResponse() { + return new CleanupRepositoryResponse(RepositoryCleanupResult.start().finish()); } @Override - protected AcknowledgedResponse read(StreamInput in) throws IOException { - return new AcknowledgedResponse(in); + protected CleanupRepositoryResponse read(StreamInput in) throws IOException { + return new CleanupRepositoryResponse(in); } @Override protected void masterOperation(Task task, CleanupRepositoryRequest request, ClusterState state, - ActionListener listener) { - snapshotsService.deleteSnapshot(request.repository(), null, - ActionListener.map(listener, v -> new AcknowledgedResponse(true)), false); + ActionListener listener) { + snapshotsService.cleanupRepo(request.repository(), ActionListener.map(listener, CleanupRepositoryResponse::new)); } @Override diff --git a/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java b/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java index a5f03dbb0e175..fdee39fdb1f93 100644 --- a/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java +++ b/server/src/main/java/org/elasticsearch/client/ClusterAdminClient.java @@ -51,6 +51,7 @@ import org.elasticsearch.action.admin.cluster.node.usage.NodesUsageResponse; import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequestBuilder; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequestBuilder; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesRequest; @@ -463,12 +464,12 @@ public interface ClusterAdminClient extends ElasticsearchClient { /** * Cleans up repository. */ - ActionFuture cleanupRepository(CleanupRepositoryRequest repository); + ActionFuture cleanupRepository(CleanupRepositoryRequest repository); /** * Cleans up repository. */ - void cleanupRepository(CleanupRepositoryRequest repository, ActionListener listener); + void cleanupRepository(CleanupRepositoryRequest repository, ActionListener listener); /** * Verifies a repository. diff --git a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java index 46ef92720b384..283b8dc0a2843 100644 --- a/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java +++ b/server/src/main/java/org/elasticsearch/client/support/AbstractClient.java @@ -67,6 +67,7 @@ import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequestBuilder; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryAction; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequestBuilder; @@ -1013,12 +1014,12 @@ public CleanupRepositoryRequestBuilder prepareCleanupRepository(String repositor } @Override - public ActionFuture cleanupRepository(CleanupRepositoryRequest request) { + public ActionFuture cleanupRepository(CleanupRepositoryRequest request) { return execute(CleanupRepositoryAction.INSTANCE, request); } @Override - public void cleanupRepository(CleanupRepositoryRequest request, ActionListener listener) { + public void cleanupRepository(CleanupRepositoryRequest request, ActionListener listener) { execute(CleanupRepositoryAction.INSTANCE, request, listener); } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java index 0fdbfe4d08293..89fb30f05ff85 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.repositories; +import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ToXContentObject; @@ -37,6 +38,11 @@ private RepositoryCleanupResult(long bytes, long blobs) { this.blobs = blobs; } + public RepositoryCleanupResult(StreamInput in) throws IOException { + bytes = in.readLong(); + blobs = in.readLong(); + } + public static Progress start() { return new Progress(); } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 44ba185713b03..a18b279c660f1 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -81,6 +81,7 @@ import org.elasticsearch.indices.recovery.RecoveryState; import org.elasticsearch.repositories.IndexId; import org.elasticsearch.repositories.Repository; +import org.elasticsearch.repositories.RepositoryCleanupResult; import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.RepositoryException; import org.elasticsearch.repositories.RepositoryVerificationException; @@ -437,13 +438,13 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action * @param repositoryStateId Current repository state id * @param listener Lister to complete when done */ - public void cleanup(long repositoryStateId, ActionListener listener) { + public void cleanup(long repositoryStateId, ActionListener listener) { ActionListener.completeWith(listener, () -> { // TODO: ensure state id final Map foundIndices = blobStore().blobContainer(indicesPath()).children(); final RepositoryData repositoryData = repositoryData(repositoryStateId); cleanupStaleIndices(foundIndices, repositoryData.getIndices()); - return null; + return RepositoryCleanupResult.start().finish(); }); } diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 133107a75ce17..d9d63a182c88c 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -66,6 +66,7 @@ import org.elasticsearch.repositories.IndexId; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.repositories.Repository; +import org.elasticsearch.repositories.RepositoryCleanupResult; import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.RepositoryMissingException; import org.elasticsearch.repositories.blobstore.BlobStoreRepository; @@ -1094,14 +1095,10 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS * @param snapshotName snapshotName * @param listener listener */ - public void deleteSnapshot(final String repositoryName, @Nullable String snapshotName, final ActionListener listener, + public void deleteSnapshot(final String repositoryName, final String snapshotName, final ActionListener listener, final boolean immediatePriority) { // First, look for the snapshot in the repository final Repository repository = repositoriesService.repository(repositoryName); - if (snapshotName == null && repository instanceof BlobStoreRepository == false) { - listener.onFailure(new IllegalArgumentException("Repository [" + repositoryName + "] does not support repository cleanup")); - return; - } final RepositoryData repositoryData = repository.getRepositoryData(); final Optional incompatibleSnapshotId = repositoryData.getIncompatibleSnapshotIds().stream().filter(s -> snapshotName.equals(s.getName())).findFirst(); @@ -1115,29 +1112,32 @@ public void deleteSnapshot(final String repositoryName, @Nullable String snapsho // if nothing found by the same name, then look in the cluster state for current in progress snapshots long repoGenId = repositoryData.getGenId(); if (matchedEntry.isPresent() == false) { - final List currentInProgress = currentSnapshots(repositoryName, Collections.emptyList()); - Optional matchedInProgress = currentInProgress.stream() + Optional matchedInProgress = currentSnapshots(repositoryName, Collections.emptyList()).stream() .filter(s -> s.snapshot().getSnapshotId().getName().equals(snapshotName)).findFirst(); if (matchedInProgress.isPresent()) { matchedEntry = matchedInProgress.map(s -> s.snapshot().getSnapshotId()); // Derive repository generation if a snapshot is in progress because it will increment the generation when it finishes repoGenId = matchedInProgress.get().getRepositoryStateId() + 1L; - } else if (currentInProgress.isEmpty() == false) { - repoGenId = currentInProgress.get(0).getRepositoryStateId() + 1L; } } - if (snapshotName != null && matchedEntry.isPresent() == false) { + if (matchedEntry.isPresent() == false) { throw new SnapshotMissingException(repositoryName, snapshotName); } - if (matchedEntry.isPresent()) { - deleteSnapshot(new Snapshot(repositoryName, matchedEntry.get()), listener, repoGenId, immediatePriority); - } else { - cleanupRepo(repositoryName, listener, repoGenId, immediatePriority); + deleteSnapshot(new Snapshot(repositoryName, matchedEntry.get()), listener, repoGenId, immediatePriority); + } + + public void cleanupRepo(final String repositoryName, final ActionListener listener) { + // First, look for the snapshot in the repository + final Repository repository = repositoriesService.repository(repositoryName); + if (repository instanceof BlobStoreRepository == false) { + listener.onFailure(new IllegalArgumentException("Repository [" + repositoryName + "] does not support repository cleanup")); + return; } + cleanupRepo(repositoryName, listener, repository.getRepositoryData().getGenId()); } - public void cleanupRepo(String repositoryName, ActionListener listener, long repositoryStateId, boolean immediatePriority) { - Priority priority = immediatePriority ? Priority.IMMEDIATE : Priority.NORMAL; + private void cleanupRepo(String repositoryName, ActionListener listener, long repositoryStateId) { + Priority priority = Priority.NORMAL; clusterService.submitStateUpdateTask("delete snapshot", new ClusterStateUpdateTask(priority) { @Override public ClusterState execute(ClusterState currentState) { @@ -1168,7 +1168,7 @@ public ClusterState execute(ClusterState currentState) { @Override public void onFailure(String source, Exception e) { - after(e); + after(e, null); } @Override @@ -1179,13 +1179,13 @@ protected void doRun() { Repository repository = repositoriesService.repository(repositoryName); assert repository instanceof BlobStoreRepository; ((BlobStoreRepository) repository) - .cleanup(repositoryStateId, ActionListener.wrap(v -> after(null), e -> after(e))); + .cleanup(repositoryStateId, ActionListener.wrap(result -> after(null, result), e -> after(e, null))); } }); } - private void after(@Nullable Exception e) { - removeSnapshotDeletionFromClusterState(null, e, listener); + private void after(@Nullable Exception e, @Nullable RepositoryCleanupResult result) { + removeSnapshotDeletionFromClusterState(null, e, ActionListener.delegateFailure(listener, (l, r) -> l.onResponse(result))); } }); } From 019b58fa33b69b8997ed0a46add4889c77d91912 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 14:43:43 +0200 Subject: [PATCH 20/66] compiles --- .../blobstore/url/URLBlobContainer.java | 3 +- .../azure/AzureBlobContainer.java | 5 ++- .../repositories/azure/AzureBlobStore.java | 6 ++- .../azure/AzureStorageService.java | 12 +++++- .../gcs/GoogleCloudStorageBlobContainer.java | 5 ++- .../gcs/GoogleCloudStorageBlobStore.java | 8 +++- .../repositories/hdfs/HdfsBlobContainer.java | 5 ++- .../repositories/s3/S3BlobContainer.java | 10 +++-- .../cleanup/CleanupRepositoryResponse.java | 4 ++ .../common/blobstore/BlobContainer.java | 3 +- .../common/blobstore/fs/FsBlobContainer.java | 21 ++++++++-- .../repositories/RepositoryCleanupResult.java | 19 +++++++--- .../blobstore/BlobStoreRepository.java | 15 +++++--- .../mockstore/BlobContainerWrapper.java | 5 ++- .../snapshots/mockstore/MockRepository.java | 5 ++- .../AbstractThirdPartyRepositoryTestCase.java | 38 +++++++++++++------ 16 files changed, 120 insertions(+), 44 deletions(-) diff --git a/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java b/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java index f9406e454474f..e6432e43a26a6 100644 --- a/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java +++ b/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java @@ -35,6 +35,7 @@ import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.Map; +import java.util.function.LongConsumer; /** * URL blob implementation of {@link org.elasticsearch.common.blobstore.BlobContainer} @@ -97,7 +98,7 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete() { + public void delete(LongConsumer resultConsumer) { throw new UnsupportedOperationException("URL repository is read only"); } diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java index 12113542dee44..ffe0bcbaaa34b 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java @@ -42,6 +42,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ExecutorService; +import java.util.function.LongConsumer; public class AzureBlobContainer extends AbstractBlobContainer { @@ -127,9 +128,9 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete() throws IOException { + public void delete(LongConsumer resultConsumer) throws IOException { try { - blobStore.deleteBlobDirectory(keyPath, threadPool.executor(AzureRepositoryPlugin.REPOSITORY_THREAD_POOL_NAME)); + blobStore.deleteBlobDirectory(keyPath, threadPool.executor(AzureRepositoryPlugin.REPOSITORY_THREAD_POOL_NAME), resultConsumer); } catch (URISyntaxException | StorageException e) { throw new IOException(e); } diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java index a7d9bb93a5125..f5639b53f52bc 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java @@ -38,6 +38,7 @@ import java.util.Map; import java.util.concurrent.Executor; import java.util.function.Function; +import java.util.function.LongConsumer; import java.util.stream.Collectors; import static java.util.Collections.emptyMap; @@ -92,8 +93,9 @@ public void deleteBlob(String blob) throws URISyntaxException, StorageException service.deleteBlob(clientName, container, blob); } - public void deleteBlobDirectory(String path, Executor executor) throws URISyntaxException, StorageException, IOException { - service.deleteBlobDirectory(clientName, container, path, executor); + public void deleteBlobDirectory(String path, Executor executor, LongConsumer resultConsumer) + throws URISyntaxException, StorageException, IOException { + service.deleteBlobDirectory(clientName, container, path, executor, resultConsumer); } public InputStream getInputStream(String blob) throws URISyntaxException, StorageException, IOException { diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java index f4ee7b9dbcad9..a49de0be332f1 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java @@ -67,6 +67,7 @@ import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.LongConsumer; import java.util.function.Supplier; import static java.util.Collections.emptyMap; @@ -192,7 +193,7 @@ public void deleteBlob(String account, String container, String blob) throws URI }); } - void deleteBlobDirectory(String account, String container, String path, Executor executor) + void deleteBlobDirectory(String account, String container, String path, Executor executor, LongConsumer resultConsumer) throws URISyntaxException, StorageException, IOException { final Tuple> client = client(account); final CloudBlobContainer blobContainer = client.v1().getContainerReference(container); @@ -208,7 +209,16 @@ void deleteBlobDirectory(String account, String container, String path, Executor executor.execute(new AbstractRunnable() { @Override protected void doRun() throws Exception { + final long len; + if (blobItem instanceof CloudBlob) { + len = ((CloudBlob) blobItem).getProperties().getLength(); + } else { + len = -1L; + } deleteBlob(account, container, blobPath); + if (len >= 0) { + resultConsumer.accept(len); + } } @Override diff --git a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java index 75d4ad92fbf8e..083014b90cb99 100644 --- a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java +++ b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java @@ -29,6 +29,7 @@ import java.io.InputStream; import java.util.List; import java.util.Map; +import java.util.function.LongConsumer; import java.util.stream.Collectors; class GoogleCloudStorageBlobContainer extends AbstractBlobContainer { @@ -87,8 +88,8 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete() throws IOException { - blobStore.deleteDirectory(path().buildAsString()); + public void delete(LongConsumer resultConsumer) throws IOException { + blobStore.deleteDirectory(path().buildAsString(), resultConsumer); } @Override diff --git a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java index 2477fd2962bde..3549880348f5a 100644 --- a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java +++ b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java @@ -56,6 +56,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.LongConsumer; import java.util.stream.Collectors; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; @@ -312,12 +313,15 @@ void deleteBlob(String blobName) throws IOException { * * @param pathStr Name of path to delete */ - void deleteDirectory(String pathStr) throws IOException { + void deleteDirectory(String pathStr, LongConsumer resultConsumer) throws IOException { SocketAccess.doPrivilegedVoidIOException(() -> { Page page = client().get(bucketName).list(BlobListOption.prefix(pathStr)); do { final Collection blobsToDelete = new ArrayList<>(); - page.getValues().forEach(b -> blobsToDelete.add(b.getName())); + page.getValues().forEach(b -> { + blobsToDelete.add(b.getName()); + resultConsumer.accept(b.getSize()); + }); deleteBlobsIgnoringIfNotExists(blobsToDelete); page = page.getNextPage(); } while (page != null); diff --git a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java index b050645f9952c..af298131cd583 100644 --- a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java +++ b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java @@ -43,6 +43,7 @@ import java.util.EnumSet; import java.util.LinkedHashMap; import java.util.Map; +import java.util.function.LongConsumer; final class HdfsBlobContainer extends AbstractBlobContainer { private final HdfsBlobStore store; @@ -79,7 +80,9 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete() throws IOException { + public void delete(LongConsumer resultConsumer) throws IOException { + // TODO: See if we can get precise result reporting. + resultConsumer.accept(0); store.execute(fileContext -> fileContext.delete(path, true)); } diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java index c2e1f3de7f0dd..73f171ef3fb78 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java @@ -56,6 +56,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.LongConsumer; import java.util.stream.Collectors; import static java.util.Map.entry; @@ -131,7 +132,7 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete() throws IOException { + public void delete(LongConsumer resultConsumer) throws IOException { try (AmazonS3Reference clientReference = blobStore.clientReference()) { ObjectListing prevListing = null; while (true) { @@ -145,8 +146,11 @@ public void delete() throws IOException { listObjectsRequest.setPrefix(keyPath); list = SocketAccess.doPrivileged(() -> clientReference.client().listObjects(listObjectsRequest)); } - final List blobsToDelete = - list.getObjectSummaries().stream().map(S3ObjectSummary::getKey).collect(Collectors.toList()); + final List blobsToDelete = new ArrayList<>(); + list.getObjectSummaries().forEach(s3ObjectSummary -> { + resultConsumer.accept(s3ObjectSummary.getSize()); + blobsToDelete.add(s3ObjectSummary.getKey()); + }); if (list.isTruncated()) { doDeleteBlobs(blobsToDelete, false); prevListing = list; diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java index fa581860f3a48..98aafa0e41af9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java @@ -39,6 +39,10 @@ public CleanupRepositoryResponse(StreamInput in) throws IOException { result = new RepositoryCleanupResult(in); } + public RepositoryCleanupResult result() { + return result; + } + @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java index a44d1fb05308a..e72a70d0e8b10 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java @@ -25,6 +25,7 @@ import java.nio.file.NoSuchFileException; import java.util.List; import java.util.Map; +import java.util.function.LongConsumer; /** * An interface for managing a repository of blob entries, where each blob entry is just a named group of bytes. @@ -113,7 +114,7 @@ public interface BlobContainer { * Deletes this container and all its contents from the repository. * @throws IOException on failure */ - void delete() throws IOException; + void delete(LongConsumer resultConsumer) throws IOException; /** * Deletes the blobs with given names. Unlike {@link #deleteBlob(String)} this method will not throw an exception diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java b/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java index b51115b246673..7cacba309b93b 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java @@ -45,6 +45,7 @@ import java.nio.file.attribute.BasicFileAttributes; import java.util.HashMap; import java.util.Map; +import java.util.function.LongConsumer; import static java.util.Collections.unmodifiableMap; @@ -110,7 +111,7 @@ public void deleteBlob(String blobName) throws IOException { if (Files.isDirectory(blobPath)) { // delete directory recursively as long as it is empty (only contains empty directories), // which is the reason we aren't deleting any files, only the directories on the post-visit - Files.walkFileTree(blobPath, new SimpleFileVisitor() { + Files.walkFileTree(blobPath, new SimpleFileVisitor<>() { @Override public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { Files.delete(dir); @@ -123,8 +124,22 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx } @Override - public void delete() throws IOException { - IOUtils.rm(path); + public void delete(LongConsumer resultConsumer) throws IOException { + Files.walkFileTree(path, new SimpleFileVisitor<>() { + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException impossible) throws IOException { + assert impossible == null; + Files.delete(dir); + return FileVisitResult.CONTINUE; + } + + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + Files.delete(file); + resultConsumer.accept(attrs.size()); + return FileVisitResult.CONTINUE; + } + }); } @Override diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java index 89fb30f05ff85..dce6037465c86 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java @@ -25,7 +25,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; -import java.util.stream.LongStream; +import java.util.function.LongConsumer; public final class RepositoryCleanupResult implements Writeable, ToXContentObject { @@ -43,6 +43,14 @@ public RepositoryCleanupResult(StreamInput in) throws IOException { blobs = in.readLong(); } + public long bytes() { + return bytes; + } + + public long blobs() { + return blobs; + } + public static Progress start() { return new Progress(); } @@ -58,16 +66,17 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder.startObject().field("bytes", bytes).field("blobs", blobs).endObject(); } - public static final class Progress { + public static final class Progress implements LongConsumer { private long bytesCounter; private long blobsCounter; - public void add(long[] sizes) { + @Override + public void accept(long size) { synchronized (this) { - blobsCounter += sizes.length; - bytesCounter += LongStream.of(sizes).sum(); + ++blobsCounter; + bytesCounter += size; } } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index a18b279c660f1..30eb229d1c95b 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -107,6 +107,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.LongConsumer; import java.util.stream.Collectors; import static org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo.canonicalName; @@ -421,7 +422,7 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action .orElse(Collections.emptyList()), snapshotId, ActionListener.map(listener, v -> { - cleanupStaleIndices(foundIndices, survivingIndices); + cleanupStaleIndices(foundIndices, survivingIndices, l -> {}); return null; }) ); @@ -433,7 +434,7 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action * TODO: Add top level blob cleanup * TODO: Add shard level cleanups *
    - *
  • Deleting stale indices {@link #cleanupStaleIndices(Map, Map)}
    • + *
  • Deleting stale indices {@link #cleanupStaleIndices}
    • *
* @param repositoryStateId Current repository state id * @param listener Lister to complete when done @@ -443,12 +444,14 @@ public void cleanup(long repositoryStateId, ActionListener foundIndices = blobStore().blobContainer(indicesPath()).children(); final RepositoryData repositoryData = repositoryData(repositoryStateId); - cleanupStaleIndices(foundIndices, repositoryData.getIndices()); - return RepositoryCleanupResult.start().finish(); + final RepositoryCleanupResult.Progress progress = RepositoryCleanupResult.start(); + cleanupStaleIndices(foundIndices, repositoryData.getIndices(), progress); + return progress.finish(); }); } - private void cleanupStaleIndices(Map foundIndices, Map survivingIndices) { + private void cleanupStaleIndices(Map foundIndices, Map survivingIndices, + LongConsumer progress) { try { final Set survivingIndexIds = survivingIndices.values().stream() .map(IndexId::getId).collect(Collectors.toSet()); @@ -457,7 +460,7 @@ private void cleanupStaleIndices(Map foundIndices, Map(future) { @Override protected void doRun() throws Exception { - repo.blobStore().blobContainer(path).delete(); + repo.blobStore().blobContainer(path).delete(l -> {}); future.onResponse(null); } }); @@ -226,26 +227,41 @@ public void testCleanup() throws Exception { .state(), equalTo(SnapshotState.SUCCESS)); - logger.info("--> creating a dangling index folder"); final BlobStoreRepository repo = (BlobStoreRepository) getInstanceFromNode(RepositoriesService.class).repository("test-repo"); - final PlainActionFuture future = PlainActionFuture.newFuture(); final Executor genericExec = repo.threadPool().executor(ThreadPool.Names.GENERIC); + + logger.info("--> creating a dangling index folder"); + + createDanglingIndex(repo, genericExec); + + logger.info("--> deleting a snapshot to trigger repository cleanup"); + client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest("test-repo", snapshotName)).actionGet(); + + assertConsistentRepository(repo, genericExec); + + logger.info("--> Create dangling index"); + createDanglingIndex(repo, genericExec); + + logger.info("--> Execute repository cleanup"); + final CleanupRepositoryResponse response = client().admin().cluster().prepareCleanupRepository("test-repo").get(); + assertThat(response.result().blobs(), equalTo(1L)); + assertThat(response.result().bytes(), equalTo(3L)); + } + + private void createDanglingIndex(final BlobStoreRepository repo, final Executor genericExec) throws Exception { + final PlainActionFuture future = PlainActionFuture.newFuture(); genericExec.execute(new ActionRunnable<>(future) { @Override protected void doRun() throws Exception { final BlobStore blobStore = repo.blobStore(); - blobStore.blobContainer(BlobPath.cleanPath().add("indices").add("foo")) - .writeBlob("bar", new ByteArrayInputStream(new byte[0]), 0, false); + blobStore.blobContainer(repo.basePath().add("indices").add("foo")) + .writeBlob("bar", new ByteArrayInputStream(new byte[3]), 3, false); future.onResponse(null); } }); future.actionGet(); assertTrue(assertCorruptionVisible(repo, genericExec)); - logger.info("--> deleting a snapshot to trigger repository cleanup"); - client().admin().cluster().deleteSnapshot(new DeleteSnapshotRequest("test-repo", snapshotName)).actionGet(); - - assertConsistentRepository(repo, genericExec); } protected boolean assertCorruptionVisible(BlobStoreRepository repo, Executor executor) throws Exception { @@ -255,8 +271,8 @@ protected boolean assertCorruptionVisible(BlobStoreRepository repo, Executor exe protected void doRun() throws Exception { final BlobStore blobStore = repo.blobStore(); future.onResponse( - blobStore.blobContainer(BlobPath.cleanPath().add("indices")).children().containsKey("foo") - && blobStore.blobContainer(BlobPath.cleanPath().add("indices").add("foo")).blobExists("bar") + blobStore.blobContainer(repo.basePath().add("indices")).children().containsKey("foo") + && blobStore.blobContainer(repo.basePath().add("indices").add("foo")).blobExists("bar") ); } }); From 7f4186038ec6b6e3dc5b500fdec9bbdfe3320307 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 15:18:52 +0200 Subject: [PATCH 21/66] fix hdfs repo tests --- .../repositories/hdfs/HdfsRepositoryTests.java | 12 ++++++++++++ .../AbstractThirdPartyRepositoryTestCase.java | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/plugins/repository-hdfs/src/test/java/org/elasticsearch/repositories/hdfs/HdfsRepositoryTests.java b/plugins/repository-hdfs/src/test/java/org/elasticsearch/repositories/hdfs/HdfsRepositoryTests.java index e34f290a8e299..d65db92f0670f 100644 --- a/plugins/repository-hdfs/src/test/java/org/elasticsearch/repositories/hdfs/HdfsRepositoryTests.java +++ b/plugins/repository-hdfs/src/test/java/org/elasticsearch/repositories/hdfs/HdfsRepositoryTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.repositories.hdfs; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.bootstrap.JavaVersion; import org.elasticsearch.common.settings.MockSecureSettings; @@ -30,6 +31,7 @@ import java.util.Collection; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; @ThreadLeakFilters(filters = HdfsClientThreadLeakFilter.class) public class HdfsRepositoryTests extends AbstractThirdPartyRepositoryTestCase { @@ -58,4 +60,14 @@ protected void createRepository(String repoName) { ).get(); assertThat(putRepositoryResponse.isAcknowledged(), equalTo(true)); } + + // HDFS repository doesn't have precise cleanup stats so we only check whether or not any blobs were removed + @Override + protected void assertCleanupResponse(CleanupRepositoryResponse response, long bytes, long blobs) { + if (blobs > 0) { + assertThat(response.result().blobs(), greaterThan(0L)); + } else { + assertThat(response.result().blobs(), equalTo(0L)); + } + } } diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java index e39207cdc4d80..5ad8091609ac0 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java @@ -245,6 +245,10 @@ public void testCleanup() throws Exception { logger.info("--> Execute repository cleanup"); final CleanupRepositoryResponse response = client().admin().cluster().prepareCleanupRepository("test-repo").get(); + assertCleanupResponse(response, 3L, 1L); + } + + protected void assertCleanupResponse(CleanupRepositoryResponse response, long bytes, long blobs) { assertThat(response.result().blobs(), equalTo(1L)); assertThat(response.result().bytes(), equalTo(3L)); } From 8dbd0662a1dc510b57a9a6e4086649b93ea63370 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 15:28:56 +0200 Subject: [PATCH 22/66] add cleanup rest test --- .../api/snapshot.cleanup_repository.json | 28 +++++++++++++++++++ .../test/snapshot.create/10_basic.yml | 6 ++++ 2 files changed, 34 insertions(+) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.cleanup_repository.json diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.cleanup_repository.json b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.cleanup_repository.json new file mode 100644 index 0000000000000..692aa4d0477d8 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.cleanup_repository.json @@ -0,0 +1,28 @@ +{ + "snapshot.cleanup_repository": { + "documentation": "http://www.elastic.co/guide/en/elasticsearch/reference/master/modules-snapshots.html", + "stability": "stable", + "methods": ["POST"], + "url": { + "paths": ["/_snapshot/{repository}/cleanup"], + "parts": { + "repository": { + "type": "string", + "required" : true, + "description": "A repository name" + } + }, + "params": { + "master_timeout": { + "type" : "time", + "description" : "Explicit operation timeout for connection to master node" + }, + "timeout": { + "type" : "time", + "description" : "Explicit operation timeout" + } + } + }, + "body": {} + } +} diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml index 0a5a7260a27a8..3deb53f284815 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml @@ -38,6 +38,12 @@ setup: - match: { acknowledged: true } + - do: + snapshot.cleanup_repository: + + - match: { result.bytes: 0 } + - match: { result.blobs: 0 } + --- "Create a snapshot for missing index": From 766c0be49f6db8de51be797af0218a501005704f Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 15:31:47 +0200 Subject: [PATCH 23/66] fix rest test --- .../resources/rest-api-spec/test/snapshot.create/10_basic.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml index 3deb53f284815..80c8dee0ed4d7 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml @@ -40,6 +40,7 @@ setup: - do: snapshot.cleanup_repository: + repository: test_repo_create_1 - match: { result.bytes: 0 } - match: { result.blobs: 0 } From 2e2df569024af38f655e682b3232c5794c6ffe83 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 15:35:00 +0200 Subject: [PATCH 24/66] fix rest test --- .../resources/rest-api-spec/test/snapshot.create/10_basic.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml index 80c8dee0ed4d7..4fef731b37b24 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml @@ -42,8 +42,8 @@ setup: snapshot.cleanup_repository: repository: test_repo_create_1 - - match: { result.bytes: 0 } - - match: { result.blobs: 0 } + - match: { results.bytes: 0 } + - match: { results.blobs: 0 } --- "Create a snapshot for missing index": From 65a5320634e75128ac02a83b12bffdc450c87d4e Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 15:53:06 +0200 Subject: [PATCH 25/66] shorter logic --- .../snapshots/SnapshotsService.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index d9d63a182c88c..c2bde85b45a07 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -1126,19 +1126,21 @@ public void deleteSnapshot(final String repositoryName, final String snapshotNam deleteSnapshot(new Snapshot(repositoryName, matchedEntry.get()), listener, repoGenId, immediatePriority); } - public void cleanupRepo(final String repositoryName, final ActionListener listener) { + /** + * Runs cleanup operations on the given repository. + * + * @param repositoryName Repository to clean up + * @param listener Listener for cleanup result + */ + public void cleanupRepo(String repositoryName, ActionListener listener) { // First, look for the snapshot in the repository final Repository repository = repositoriesService.repository(repositoryName); if (repository instanceof BlobStoreRepository == false) { listener.onFailure(new IllegalArgumentException("Repository [" + repositoryName + "] does not support repository cleanup")); return; } - cleanupRepo(repositoryName, listener, repository.getRepositoryData().getGenId()); - } - - private void cleanupRepo(String repositoryName, ActionListener listener, long repositoryStateId) { - Priority priority = Priority.NORMAL; - clusterService.submitStateUpdateTask("delete snapshot", new ClusterStateUpdateTask(priority) { + final long repositoryStateId = repository.getRepositoryData().getGenId(); + clusterService.submitStateUpdateTask("delete snapshot", new ClusterStateUpdateTask() { @Override public ClusterState execute(ClusterState currentState) { SnapshotDeletionsInProgress deletionsInProgress = currentState.custom(SnapshotDeletionsInProgress.TYPE); From 3cb590e568efc7e8c7ebf1e27431d664c903844b Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 20:58:23 +0200 Subject: [PATCH 26/66] HLRC integration --- .../elasticsearch/client/SnapshotClient.java | 31 +++++++++++++++++++ .../client/SnapshotRequestConverters.java | 15 +++++++++ .../org/elasticsearch/client/SnapshotIT.java | 15 ++++++++- .../cleanup/CleanupRepositoryResponse.java | 22 +++++++++++-- .../repositories/RepositoryCleanupResult.java | 12 +++++-- 5 files changed, 90 insertions(+), 5 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java index f3a49f064596e..56344bf55c3db 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java @@ -20,6 +20,8 @@ package org.elasticsearch.client; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesRequest; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesResponse; @@ -170,6 +172,35 @@ public void verifyRepositoryAsync(VerifyRepositoryRequest verifyRepositoryReques VerifyRepositoryResponse::fromXContent, listener, emptySet()); } + /** + * Cleans up a snapshot repository. + * See Snapshot and Restore + * API on elastic.co + * @param cleanupRepositoryRequest the request + * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized + * @return the response + * @throws IOException in case there is a problem sending the request or parsing back the response + */ + public CleanupRepositoryResponse cleanupRepository(CleanupRepositoryRequest cleanupRepositoryRequest, RequestOptions options) + throws IOException { + return restHighLevelClient.performRequestAndParseEntity(cleanupRepositoryRequest, SnapshotRequestConverters::cleanupRepository, + options, CleanupRepositoryResponse::fromXContent, emptySet()); + } + + /** + * Asynchronously cleans up a snapshot repository. + * See Snapshot and Restore + * API on elastic.co + * @param cleanupRepositoryRequest the request + * @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized + * @param listener the listener to be notified upon request completion + */ + public void cleanupRepositoryAsync(CleanupRepositoryRequest cleanupRepositoryRequest, RequestOptions options, + ActionListener listener) { + restHighLevelClient.performRequestAsyncAndParseEntity(cleanupRepositoryRequest, SnapshotRequestConverters::cleanupRepository, + options, CleanupRepositoryResponse::fromXContent, listener, emptySet()); + } + /** * Creates a snapshot. *

diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotRequestConverters.java index 406470ea52cda..7829637bffb51 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotRequestConverters.java @@ -23,6 +23,7 @@ import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesRequest; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest; @@ -94,6 +95,20 @@ static Request verifyRepository(VerifyRepositoryRequest verifyRepositoryRequest) return request; } + static Request cleanupRepository(CleanupRepositoryRequest cleanupRepositoryRequest) { + String endpoint = new RequestConverters.EndpointBuilder().addPathPartAsIs("_snapshot") + .addPathPart(cleanupRepositoryRequest.repository()) + .addPathPartAsIs("cleanup") + .build(); + Request request = new Request(HttpPost.METHOD_NAME, endpoint); + + RequestConverters.Params parameters = new RequestConverters.Params(); + parameters.withMasterTimeout(cleanupRepositoryRequest.masterNodeTimeout()); + parameters.withTimeout(cleanupRepositoryRequest.timeout()); + request.addParameters(parameters.asMap()); + return request; + } + static Request createSnapshot(CreateSnapshotRequest createSnapshotRequest) throws IOException { String endpoint = new RequestConverters.EndpointBuilder().addPathPart("_snapshot") .addPathPart(createSnapshotRequest.repository()) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java index 8e4001442b0cc..6664535e5f3f5 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java @@ -20,6 +20,8 @@ package org.elasticsearch.client; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryRequest; +import org.elasticsearch.action.admin.cluster.repositories.cleanup.CleanupRepositoryResponse; import org.elasticsearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesRequest; import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesResponse; @@ -133,6 +135,17 @@ public void testVerifyRepository() throws IOException { assertThat(response.getNodes().size(), equalTo(1)); } + public void testCleanupRepository() throws IOException { + AcknowledgedResponse putRepositoryResponse = createTestRepository("test", FsRepository.TYPE, "{\"location\": \".\"}"); + assertTrue(putRepositoryResponse.isAcknowledged()); + + CleanupRepositoryRequest request = new CleanupRepositoryRequest("test"); + CleanupRepositoryResponse response = execute(request, highLevelClient().snapshot()::cleanupRepository, + highLevelClient().snapshot()::cleanupRepositoryAsync); + assertThat(response.result().bytes(), equalTo(0)); + assertThat(response.result().blobs(), equalTo(0)); + } + public void testCreateSnapshot() throws IOException { String repository = "test_repository"; assertTrue(createTestRepository(repository, FsRepository.TYPE, "{\"location\": \".\"}").isAcknowledged()); @@ -317,4 +330,4 @@ private static Map randomUserMetadata() { } return metadata; } -} \ No newline at end of file +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java index 98aafa0e41af9..30bf05796a812 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java @@ -19,17 +19,31 @@ package org.elasticsearch.action.admin.cluster.repositories.cleanup; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.repositories.RepositoryCleanupResult; import java.io.IOException; public final class CleanupRepositoryResponse extends ActionResponse implements ToXContentObject { - private final RepositoryCleanupResult result; + private static final ObjectParser PARSER = + new ObjectParser<>(CleanupRepositoryResponse.class.getName(), true, CleanupRepositoryResponse::new); + + static { + PARSER.declareObject((response, cleanupResult) -> response.result = cleanupResult, + RepositoryCleanupResult.PARSER, new ParseField("snapshot")); + } + + private RepositoryCleanupResult result; + + public CleanupRepositoryResponse() { + } public CleanupRepositoryResponse(RepositoryCleanupResult result) { this.result = result; @@ -44,11 +58,15 @@ public RepositoryCleanupResult result() { } @Override - public void writeTo(StreamOutput out) throws IOException { + public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); result.writeTo(out); } + public static CleanupRepositoryResponse fromXContent(XContentParser parser) { + return PARSER.apply(parser, null); + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject().field("results"); diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java index dce6037465c86..c0584b3a3ae40 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java @@ -21,6 +21,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -29,9 +30,16 @@ public final class RepositoryCleanupResult implements Writeable, ToXContentObject { - private final long bytes; + public static final ObjectParser PARSER = + new ObjectParser<>(RepositoryCleanupResult.class.getName(), true, RepositoryCleanupResult::new); - private final long blobs; + private long bytes; + + private long blobs; + + private RepositoryCleanupResult() { + this(0L, 0L); + } private RepositoryCleanupResult(long bytes, long blobs) { this.bytes = bytes; From 0e17678df30dd0ad2062416d210f397430df89b4 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 21:08:49 +0200 Subject: [PATCH 27/66] fix empty repo handling --- .../blobstore/BlobStoreRepository.java | 58 ++++++++++--------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 8ec16bd53a7a0..f991048996222 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -674,35 +674,39 @@ public RepositoryData getRepositoryData() { } private RepositoryData repositoryData(long indexGen) throws IOException { - final String snapshotsIndexBlobName = INDEX_FILE_PREFIX + Long.toString(indexGen); - - RepositoryData repositoryData; - // EMPTY is safe here because RepositoryData#fromXContent calls namedObject - try (InputStream blob = blobContainer().readBlob(snapshotsIndexBlobName); - XContentParser parser = XContentType.JSON.xContent().createParser(NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, blob)) { - repositoryData = RepositoryData.snapshotsFromXContent(parser, indexGen); - } + if (indexGen < 0) { + assert indexGen == RepositoryData.EMPTY_REPO_GEN; + return RepositoryData.EMPTY; + } + final String snapshotsIndexBlobName = INDEX_FILE_PREFIX + Long.toString(indexGen); + + RepositoryData repositoryData; + // EMPTY is safe here because RepositoryData#fromXContent calls namedObject + try (InputStream blob = blobContainer().readBlob(snapshotsIndexBlobName); + XContentParser parser = XContentType.JSON.xContent().createParser(NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, blob)) { + repositoryData = RepositoryData.snapshotsFromXContent(parser, indexGen); + } - // now load the incompatible snapshot ids, if they exist - try (InputStream blob = blobContainer().readBlob(INCOMPATIBLE_SNAPSHOTS_BLOB); - XContentParser parser = XContentType.JSON.xContent().createParser(NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, blob)) { - repositoryData = repositoryData.incompatibleSnapshotsFromXContent(parser); - } catch (NoSuchFileException e) { - if (isReadOnly()) { - logger.debug("[{}] Incompatible snapshots blob [{}] does not exist, the likely " + - "reason is that there are no incompatible snapshots in the repository", - metadata.name(), INCOMPATIBLE_SNAPSHOTS_BLOB); - } else { - // write an empty incompatible-snapshots blob - we do this so that there - // is a blob present, which helps speed up some cloud-based repositories - // (e.g. S3), which retry if a blob is missing with exponential backoff, - // delaying the read of repository data and sometimes causing a timeout - writeIncompatibleSnapshots(RepositoryData.EMPTY); - } + // now load the incompatible snapshot ids, if they exist + try (InputStream blob = blobContainer().readBlob(INCOMPATIBLE_SNAPSHOTS_BLOB); + XContentParser parser = XContentType.JSON.xContent().createParser(NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, blob)) { + repositoryData = repositoryData.incompatibleSnapshotsFromXContent(parser); + } catch (NoSuchFileException e) { + if (isReadOnly()) { + logger.debug("[{}] Incompatible snapshots blob [{}] does not exist, the likely " + + "reason is that there are no incompatible snapshots in the repository", + metadata.name(), INCOMPATIBLE_SNAPSHOTS_BLOB); + } else { + // write an empty incompatible-snapshots blob - we do this so that there + // is a blob present, which helps speed up some cloud-based repositories + // (e.g. S3), which retry if a blob is missing with exponential backoff, + // delaying the read of repository data and sometimes causing a timeout + writeIncompatibleSnapshots(RepositoryData.EMPTY); } - return repositoryData; + } + return repositoryData; } private static String testBlobPrefix(String seed) { From 2f87260fe4152df1f18ae4ad6478e66b539b2758 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 21:21:24 +0200 Subject: [PATCH 28/66] Fix HLRC integration --- .../src/test/java/org/elasticsearch/client/SnapshotIT.java | 4 ++-- .../repositories/cleanup/CleanupRepositoryResponse.java | 2 +- .../elasticsearch/repositories/RepositoryCleanupResult.java | 6 ++++++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java index 6664535e5f3f5..f9679cf5eb61c 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java @@ -142,8 +142,8 @@ public void testCleanupRepository() throws IOException { CleanupRepositoryRequest request = new CleanupRepositoryRequest("test"); CleanupRepositoryResponse response = execute(request, highLevelClient().snapshot()::cleanupRepository, highLevelClient().snapshot()::cleanupRepositoryAsync); - assertThat(response.result().bytes(), equalTo(0)); - assertThat(response.result().blobs(), equalTo(0)); + assertThat(response.result().bytes(), equalTo(0L)); + assertThat(response.result().blobs(), equalTo(0L)); } public void testCreateSnapshot() throws IOException { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java index 30bf05796a812..50847f8c42504 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryResponse.java @@ -37,7 +37,7 @@ public final class CleanupRepositoryResponse extends ActionResponse implements T static { PARSER.declareObject((response, cleanupResult) -> response.result = cleanupResult, - RepositoryCleanupResult.PARSER, new ParseField("snapshot")); + RepositoryCleanupResult.PARSER, new ParseField("results")); } private RepositoryCleanupResult result; diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java index c0584b3a3ae40..1c3988aafb46f 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.repositories; +import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -33,6 +34,11 @@ public final class RepositoryCleanupResult implements Writeable, ToXContentObjec public static final ObjectParser PARSER = new ObjectParser<>(RepositoryCleanupResult.class.getName(), true, RepositoryCleanupResult::new); + static { + PARSER.declareLong((result, bytes) -> result.bytes = bytes, new ParseField("bytes")); + PARSER.declareLong((result, blobs) -> result.blobs = blobs, new ParseField("blobs")); + } + private long bytes; private long blobs; From 63682c460e094184ce9296e6132778696c463d92 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Jul 2019 21:36:24 +0200 Subject: [PATCH 29/66] cleaner --- .../snapshots/SnapshotsService.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index c2bde85b45a07..513f223ddd276 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -1133,14 +1133,14 @@ public void deleteSnapshot(final String repositoryName, final String snapshotNam * @param listener Listener for cleanup result */ public void cleanupRepo(String repositoryName, ActionListener listener) { - // First, look for the snapshot in the repository final Repository repository = repositoriesService.repository(repositoryName); if (repository instanceof BlobStoreRepository == false) { listener.onFailure(new IllegalArgumentException("Repository [" + repositoryName + "] does not support repository cleanup")); return; } final long repositoryStateId = repository.getRepositoryData().getGenId(); - clusterService.submitStateUpdateTask("delete snapshot", new ClusterStateUpdateTask() { + final Snapshot placeHolder = new Snapshot(repositoryName, null); + clusterService.submitStateUpdateTask("cleanup repository", new ClusterStateUpdateTask() { @Override public ClusterState execute(ClusterState currentState) { SnapshotDeletionsInProgress deletionsInProgress = currentState.custom(SnapshotDeletionsInProgress.TYPE); @@ -1153,19 +1153,15 @@ public ClusterState execute(ClusterState currentState) { // However other snapshots are running - cannot continue throw new IllegalStateException("Cannot clean up - a snapshot is currently running"); } - // add the snapshot deletion to the cluster state + // add the empty snapshot deletion to the cluster state SnapshotDeletionsInProgress.Entry entry = new SnapshotDeletionsInProgress.Entry( - new Snapshot(repositoryName, null), - System.currentTimeMillis(), - repositoryStateId - ); + placeHolder, threadPool.absoluteTimeInMillis(), repositoryStateId); if (deletionsInProgress != null) { deletionsInProgress = deletionsInProgress.withAddedEntry(entry); } else { deletionsInProgress = SnapshotDeletionsInProgress.newInstance(entry); } - clusterStateBuilder.putCustom(SnapshotDeletionsInProgress.TYPE, deletionsInProgress); - return clusterStateBuilder.build(); + return clusterStateBuilder.putCustom(SnapshotDeletionsInProgress.TYPE, deletionsInProgress).build(); } @Override @@ -1187,7 +1183,8 @@ protected void doRun() { } private void after(@Nullable Exception e, @Nullable RepositoryCleanupResult result) { - removeSnapshotDeletionFromClusterState(null, e, ActionListener.delegateFailure(listener, (l, r) -> l.onResponse(result))); + removeSnapshotDeletionFromClusterState( + placeHolder, e, ActionListener.delegateFailure(listener, (l, r) -> l.onResponse(result))); } }); } From 0a5d85e44c634d3b3e476539bce5368b08759450 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 08:14:34 +0200 Subject: [PATCH 30/66] ensure no concurrent modifications --- .../repositories/blobstore/BlobStoreRepository.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index f991048996222..0c19d172b04fd 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -440,9 +440,15 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action */ public void cleanup(long repositoryStateId, ActionListener listener) { ActionListener.completeWith(listener, () -> { - // TODO: ensure state id + final RepositoryData repositoryData = getRepositoryData(); + if (repositoryData.getGenId() != repositoryStateId) { + // the index file was updated by a concurrent operation, so we were operating on stale + // repository data + throw new RepositoryException(metadata.name(), "concurrent modification of the repository, expected current generation [" + + repositoryStateId + "], actual current generation [" + repositoryData.getGenId() + + "] - possibly due to simultaneous snapshot deletion or creation requests"); + } final Map foundIndices = blobStore().blobContainer(indicesPath()).children(); - final RepositoryData repositoryData = repositoryData(repositoryStateId); final RepositoryCleanupResult.Progress progress = RepositoryCleanupResult.start(); cleanupStaleIndices(foundIndices, repositoryData.getIndices(), progress); return progress.finish(); From 21cfebc0da105bf2c715652fa4178709b384a2f5 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 08:23:52 +0200 Subject: [PATCH 31/66] better docs --- docs/reference/modules/snapshots.asciidoc | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/docs/reference/modules/snapshots.asciidoc b/docs/reference/modules/snapshots.asciidoc index e20e55ab5fc25..726b053b263b0 100644 --- a/docs/reference/modules/snapshots.asciidoc +++ b/docs/reference/modules/snapshots.asciidoc @@ -339,8 +339,23 @@ POST /_snapshot/my_repository/cleanup // CONSOLE // TEST[continued] -NOTE: Most of the cleanup actions performed by the repository cleanup functionality are automatically executed when deleting a snapshot. -If you regularly delete snapshots from a repository you likely will get little to no space savings from executing this functionality. +The response to a cleanup request looks as follows: + +[source,js] +-------------------------------------------------- +{ + "result": { + "bytes": 20, + "blobs": 5 + } +} +-------------------------------------------------- +// CONSOLE +// TEST[continued] + +Depending on the concrete repository implementation the numbers shown for bytes free as well as the number of blobs removed will either +be an approximation or an exact result. Any non-zero value for the number of blobs removed implies that unreferenced blobs were found and +subsequently cleaned up. [float] === Snapshot From 1eb2ddceb12ac354ef07b0762681e9766eec32f7 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 08:29:52 +0200 Subject: [PATCH 32/66] fix formatting --- docs/reference/modules/snapshots.asciidoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/reference/modules/snapshots.asciidoc b/docs/reference/modules/snapshots.asciidoc index 726b053b263b0..9c75b12f29a07 100644 --- a/docs/reference/modules/snapshots.asciidoc +++ b/docs/reference/modules/snapshots.asciidoc @@ -344,10 +344,10 @@ The response to a cleanup request looks as follows: [source,js] -------------------------------------------------- { - "result": { - "bytes": 20, - "blobs": 5 - } + "result": { + "bytes": 20, + "blobs": 5 + } } -------------------------------------------------- // CONSOLE From 951fc9de96927b8e699ba248a8fa60be0f55cabc Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 08:31:31 +0200 Subject: [PATCH 33/66] fix formatting --- docs/reference/modules/snapshots.asciidoc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/reference/modules/snapshots.asciidoc b/docs/reference/modules/snapshots.asciidoc index 9c75b12f29a07..77ee6ad4530fe 100644 --- a/docs/reference/modules/snapshots.asciidoc +++ b/docs/reference/modules/snapshots.asciidoc @@ -344,14 +344,13 @@ The response to a cleanup request looks as follows: [source,js] -------------------------------------------------- { - "result": { + "results": { "bytes": 20, "blobs": 5 } } -------------------------------------------------- -// CONSOLE -// TEST[continued] +// TESTRESPONSE Depending on the concrete repository implementation the numbers shown for bytes free as well as the number of blobs removed will either be an approximation or an exact result. Any non-zero value for the number of blobs removed implies that unreferenced blobs were found and From a10a4b36a08cec7a9e400057c179483cbf1abc3b Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 09:02:29 +0200 Subject: [PATCH 34/66] add javadoc --- .../java/org/elasticsearch/common/blobstore/BlobContainer.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java index e72a70d0e8b10..4f177c51351db 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java @@ -112,6 +112,9 @@ public interface BlobContainer { /** * Deletes this container and all its contents from the repository. + * @param resultConsumer Long consumer that implementations can invoke with the byte size of each deleted blob when deleting it. + * If invoking the result consumer exactly once per deleted blob is not possible for an implementation it must at + * least invoke the consumer once during the delete process if any data was deleted. * @throws IOException on failure */ void delete(LongConsumer resultConsumer) throws IOException; From dbb21af7641827316eb3a738edce8f1ea1630a4c Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 09:08:23 +0200 Subject: [PATCH 35/66] remove todo that was addressed --- .../repositories/cleanup/TransportCleanupRepositoryAction.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index e1358ece732f9..2b291ad1b200b 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -36,7 +36,6 @@ import java.io.IOException; -// TODO: add response that includes stats? public final class TransportCleanupRepositoryAction extends TransportMasterNodeAction { From e68439da4e5f8782bfb35396d0173e757808640a Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 09:08:57 +0200 Subject: [PATCH 36/66] simplify --- .../repositories/cleanup/TransportCleanupRepositoryAction.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 2b291ad1b200b..3806a83ddd69f 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -28,7 +28,6 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.repositories.RepositoryCleanupResult; import org.elasticsearch.snapshots.SnapshotsService; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; @@ -57,7 +56,7 @@ public TransportCleanupRepositoryAction(TransportService transportService, Clust @Override protected CleanupRepositoryResponse newResponse() { - return new CleanupRepositoryResponse(RepositoryCleanupResult.start().finish()); + return new CleanupRepositoryResponse(); } @Override From 6b3c0a99a5b59d3b25f02028bbe9169504818dc8 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 09:10:15 +0200 Subject: [PATCH 37/66] revert needless chagne --- .../blobstore/BlobStoreRepository.java | 67 +++++++++---------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 0c19d172b04fd..9b530a8dc0ab5 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -670,7 +670,36 @@ public void endVerification(String seed) { @Override public RepositoryData getRepositoryData() { try { - return repositoryData(latestIndexBlobId()); + final long indexGen = latestIndexBlobId(); + final String snapshotsIndexBlobName = INDEX_FILE_PREFIX + Long.toString(indexGen); + + RepositoryData repositoryData; + // EMPTY is safe here because RepositoryData#fromXContent calls namedObject + try (InputStream blob = blobContainer().readBlob(snapshotsIndexBlobName); + XContentParser parser = XContentType.JSON.xContent().createParser(NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, blob)) { + repositoryData = RepositoryData.snapshotsFromXContent(parser, indexGen); + } + + // now load the incompatible snapshot ids, if they exist + try (InputStream blob = blobContainer().readBlob(INCOMPATIBLE_SNAPSHOTS_BLOB); + XContentParser parser = XContentType.JSON.xContent().createParser(NamedXContentRegistry.EMPTY, + LoggingDeprecationHandler.INSTANCE, blob)) { + repositoryData = repositoryData.incompatibleSnapshotsFromXContent(parser); + } catch (NoSuchFileException e) { + if (isReadOnly()) { + logger.debug("[{}] Incompatible snapshots blob [{}] does not exist, the likely " + + "reason is that there are no incompatible snapshots in the repository", + metadata.name(), INCOMPATIBLE_SNAPSHOTS_BLOB); + } else { + // write an empty incompatible-snapshots blob - we do this so that there + // is a blob present, which helps speed up some cloud-based repositories + // (e.g. S3), which retry if a blob is missing with exponential backoff, + // delaying the read of repository data and sometimes causing a timeout + writeIncompatibleSnapshots(RepositoryData.EMPTY); + } + } + return repositoryData; } catch (NoSuchFileException ex) { // repository doesn't have an index blob, its a new blank repo return RepositoryData.EMPTY; @@ -679,42 +708,6 @@ public RepositoryData getRepositoryData() { } } - private RepositoryData repositoryData(long indexGen) throws IOException { - if (indexGen < 0) { - assert indexGen == RepositoryData.EMPTY_REPO_GEN; - return RepositoryData.EMPTY; - } - final String snapshotsIndexBlobName = INDEX_FILE_PREFIX + Long.toString(indexGen); - - RepositoryData repositoryData; - // EMPTY is safe here because RepositoryData#fromXContent calls namedObject - try (InputStream blob = blobContainer().readBlob(snapshotsIndexBlobName); - XContentParser parser = XContentType.JSON.xContent().createParser(NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, blob)) { - repositoryData = RepositoryData.snapshotsFromXContent(parser, indexGen); - } - - // now load the incompatible snapshot ids, if they exist - try (InputStream blob = blobContainer().readBlob(INCOMPATIBLE_SNAPSHOTS_BLOB); - XContentParser parser = XContentType.JSON.xContent().createParser(NamedXContentRegistry.EMPTY, - LoggingDeprecationHandler.INSTANCE, blob)) { - repositoryData = repositoryData.incompatibleSnapshotsFromXContent(parser); - } catch (NoSuchFileException e) { - if (isReadOnly()) { - logger.debug("[{}] Incompatible snapshots blob [{}] does not exist, the likely " + - "reason is that there are no incompatible snapshots in the repository", - metadata.name(), INCOMPATIBLE_SNAPSHOTS_BLOB); - } else { - // write an empty incompatible-snapshots blob - we do this so that there - // is a blob present, which helps speed up some cloud-based repositories - // (e.g. S3), which retry if a blob is missing with exponential backoff, - // delaying the read of repository data and sometimes causing a timeout - writeIncompatibleSnapshots(RepositoryData.EMPTY); - } - } - return repositoryData; - } - private static String testBlobPrefix(String seed) { return TESTS_FILE + seed; } From c64f97fbd5f8112d64a70ff94f0dda0c20979b30 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 09:14:07 +0200 Subject: [PATCH 38/66] remove resolved todo --- .../snapshots/DedicatedClusterSnapshotRestoreIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index dfbedb874d506..56e2e0d164929 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -486,7 +486,6 @@ public void testSnapshotWithStuckNode() throws Exception { () -> client().admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("test-snap") .execute().actionGet().getSnapshots("test-repo")); - // TODO: Replace this by repository cleanup endpoint call once that's available logger.info("--> Go through a loop of creating and deleting a snapshot to trigger repository cleanup"); client().admin().cluster().prepareCleanupRepository("test-repo").get(); From 7a05d279c9d06a8f3ee7489c56a97f8958abd401 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 09:23:01 +0200 Subject: [PATCH 39/66] ensure cleanup only runs on 8+ --- .../cleanup/TransportCleanupRepositoryAction.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 3806a83ddd69f..9f07e24d99fef 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.action.admin.cluster.repositories.cleanup; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.TransportMasterNodeAction; @@ -38,6 +39,8 @@ public final class TransportCleanupRepositoryAction extends TransportMasterNodeAction { + private static final Version MIN_VERSION = Version.V_8_0_0; + private final SnapshotsService snapshotsService; @Override @@ -67,7 +70,12 @@ protected CleanupRepositoryResponse read(StreamInput in) throws IOException { @Override protected void masterOperation(Task task, CleanupRepositoryRequest request, ClusterState state, ActionListener listener) { - snapshotsService.cleanupRepo(request.repository(), ActionListener.map(listener, CleanupRepositoryResponse::new)); + if (state.nodes().getMinNodeVersion().onOrAfter(MIN_VERSION)) { + snapshotsService.cleanupRepo(request.repository(), ActionListener.map(listener, CleanupRepositoryResponse::new)); + } else { + throw new IllegalArgumentException("Repository cleanup is only supported from version [" + MIN_VERSION + + "] but the oldest node version in the cluster is [" + state.nodes().getMinNodeVersion() + ']'); + } } @Override From 1f372df08d2e06cea258a330f8f1cd31e9ff8c07 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 13:16:47 +0200 Subject: [PATCH 40/66] skip cleanup check in bwc --- .../resources/rest-api-spec/test/snapshot.create/10_basic.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml index 4fef731b37b24..55419993b7e5d 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml @@ -38,6 +38,10 @@ setup: - match: { acknowledged: true } + - skip: + version: " - 7.99.99" + reason: cleanup introduced in 8.0 + - do: snapshot.cleanup_repository: repository: test_repo_create_1 From 1e5a0c7ae2ee8a106738a89396dbe1268c750aef Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 13:19:25 +0200 Subject: [PATCH 41/66] skip cleanup check in bwc --- .../test/snapshot.create/10_basic.yml | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml index 55419993b7e5d..42fe445fdea2a 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml @@ -38,6 +38,8 @@ setup: - match: { acknowledged: true } +--- +"Create a snapshot and clean up repository": - skip: version: " - 7.99.99" reason: cleanup introduced in 8.0 @@ -49,6 +51,38 @@ setup: - match: { results.bytes: 0 } - match: { results.blobs: 0 } + - do: + snapshot.create: + repository: test_repo_create_1 + snapshot: test_snapshot + wait_for_completion: true + + - match: { snapshot.snapshot: test_snapshot } + - match: { snapshot.state : SUCCESS } + - match: { snapshot.shards.successful: 1 } + - match: { snapshot.shards.failed : 0 } + + - do: + snapshot.cleanup_repository: + repository: test_repo_create_1 + + - match: { results.bytes: 0 } + - match: { results.blobs: 0 } + + - do: + snapshot.delete: + repository: test_repo_create_1 + snapshot: test_snapshot + + - match: { acknowledged: true } + + - do: + snapshot.cleanup_repository: + repository: test_repo_create_1 + + - match: { results.bytes: 0 } + - match: { results.blobs: 0 } + --- "Create a snapshot for missing index": From 3ee6559198d8417d461fba6bd42f40793650d2c6 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 3 Jul 2019 16:55:56 +0200 Subject: [PATCH 42/66] deal with null spots --- .../cluster/SnapshotDeletionsInProgress.java | 9 +++++++-- .../main/java/org/elasticsearch/snapshots/Snapshot.java | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java index 779f916872d05..2d4b923805267 100644 --- a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.snapshots.Snapshot; +import org.elasticsearch.snapshots.SnapshotId; import java.io.IOException; import java.util.ArrayList; @@ -156,7 +157,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public String toString() { StringBuilder builder = new StringBuilder("SnapshotDeletionsInProgress["); for (int i = 0; i < entries.size(); i++) { - builder.append(entries.get(i).getSnapshot().getSnapshotId().getName()); + final SnapshotId snapshotId = entries.get(i).getSnapshot().getSnapshotId(); + if (snapshotId == null) { + continue; + } + builder.append(snapshotId.getName()); if (i + 1 < entries.size()) { builder.append(","); } @@ -214,7 +219,7 @@ public boolean equals(Object o) { return false; } Entry that = (Entry) o; - return Objects.equals(snapshot, that.snapshot) + return snapshot.equals(that.snapshot) && startTime == that.startTime && repositoryStateId == that.repositoryStateId; } diff --git a/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java b/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java index e66724912888f..d55988e5d449d 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java +++ b/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java @@ -69,6 +69,7 @@ public String getRepository() { /** * Gets the snapshot id for the snapshot. */ + @Nullable public SnapshotId getSnapshotId() { return snapshotId; } From 9759b9d1ed622052a5a58b5359eb918d25015dae Mon Sep 17 00:00:00 2001 From: Armin Date: Fri, 5 Jul 2019 10:08:44 +0200 Subject: [PATCH 43/66] revert needless changes --- .../elasticsearch/cluster/SnapshotDeletionsInProgress.java | 4 ++++ .../org/elasticsearch/repositories/RepositoriesService.java | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java index 2d4b923805267..4fef31aabf66a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java @@ -45,6 +45,10 @@ public class SnapshotDeletionsInProgress extends AbstractNamedDiffable i // the list of snapshot deletion request entries private final List entries; + public SnapshotDeletionsInProgress() { + this(Collections.emptyList()); + } + private SnapshotDeletionsInProgress(List entries) { this.entries = Collections.unmodifiableList(entries); } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java index 5563d3ddeaf00..88f1051ca2769 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java @@ -434,7 +434,7 @@ private static void validate(final String repositoryName) { } - private static void ensureRepositoryNotInUse(ClusterState clusterState, String repository) { + private void ensureRepositoryNotInUse(ClusterState clusterState, String repository) { if (SnapshotsService.isRepositoryInUse(clusterState, repository) || RestoreService.isRepositoryInUse(clusterState, repository)) { throw new IllegalStateException("trying to modify or unregister repository that is currently used "); } From 58a047d0868a8e8096c24d1f73c02fc7769b62b7 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 23 Jul 2019 10:38:39 +0200 Subject: [PATCH 44/66] bck --- .../elasticsearch/cluster/ClusterModule.java | 2 + .../cluster/RepositoryCleanupInProgress.java | 124 ++++++++++++++++++ .../cluster/SnapshotDeletionsInProgress.java | 10 +- .../org/elasticsearch/snapshots/Snapshot.java | 21 +-- .../snapshots/SnapshotsService.java | 20 ++- .../MockEventuallyConsistentRepository.java | 8 +- 6 files changed, 156 insertions(+), 29 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java b/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java index d0448e2be22c9..e445615e0fc73 100644 --- a/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java +++ b/server/src/main/java/org/elasticsearch/cluster/ClusterModule.java @@ -118,6 +118,8 @@ public static List getNamedWriteables() { registerClusterCustom(entries, RestoreInProgress.TYPE, RestoreInProgress::new, RestoreInProgress::readDiffFrom); registerClusterCustom(entries, SnapshotDeletionsInProgress.TYPE, SnapshotDeletionsInProgress::new, SnapshotDeletionsInProgress::readDiffFrom); + registerClusterCustom(entries, RepositoryCleanupInProgress.TYPE, RepositoryCleanupInProgress::new, + RepositoryCleanupInProgress::readDiffFrom); // Metadata registerMetaDataCustom(entries, RepositoriesMetaData.TYPE, RepositoriesMetaData::new, RepositoriesMetaData::readDiffFrom); registerMetaDataCustom(entries, IngestMetadata.TYPE, IngestMetadata::new, IngestMetadata::readDiffFrom); diff --git a/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java b/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java new file mode 100644 index 0000000000000..8c70a75b2c06e --- /dev/null +++ b/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java @@ -0,0 +1,124 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.cluster; + +import org.elasticsearch.Version; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; + +public final class RepositoryCleanupInProgress extends AbstractNamedDiffable implements ClusterState.Custom { + + public static final String TYPE = "repository_cleanup"; + + private final List entries; + + public RepositoryCleanupInProgress(Entry... entries) { + this.entries = Arrays.asList(entries); + } + + public RepositoryCleanupInProgress(StreamInput in) throws IOException { + this.entries = in.readList(Entry::new); + } + + public static NamedDiff readDiffFrom(StreamInput in) throws IOException { + return readDiffFrom(ClusterState.Custom.class, TYPE, in); + } + + public static Entry startedEntry(String repository, long repositoryStateId) { + return new Entry(repository, repositoryStateId, false); + } + + public static Entry safeEntry(String repository, long repositoryStateId) { + return new Entry(repository, repositoryStateId, true); + } + + public boolean isSafe(String repository) { + // TODO: Should we allow parallelism across repositories here maybe? + assert repository != null; + return entries.stream().noneMatch(entry -> entry.safe == false); + } + + @Override + public String getWriteableName() { + return TYPE; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeList(entries); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startArray(TYPE); + for (Entry entry : entries) { + builder.startObject(); + { + builder.field("repository", entry.repository); + } + builder.endObject(); + } + builder.endArray(); + return builder; + } + + @Override + public String toString() { + return Strings.toString(this); + } + + @Override + public Version getMinimalSupportedVersion() { + return Version.V_8_0_0; + } + + public static final class Entry implements Writeable { + + private final String repository; + + private final long repositoryStateId; + + private final boolean safe; + + private Entry(StreamInput in) throws IOException { + repository = in.readString(); + repositoryStateId = in.readLong(); + safe = in.readBoolean(); + } + + private Entry(String repository, long repositoryStateId, boolean safe) { + this.repository = repository; + this.repositoryStateId = repositoryStateId; + this.safe = safe; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(repository); + out.writeLong(repositoryStateId); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java index 2d4b923805267..8e702fbdceea8 100644 --- a/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/SnapshotDeletionsInProgress.java @@ -27,7 +27,6 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.snapshots.Snapshot; -import org.elasticsearch.snapshots.SnapshotId; import java.io.IOException; import java.util.ArrayList; @@ -137,9 +136,6 @@ public Version getMinimalSupportedVersion() { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startArray(TYPE); for (Entry entry : entries) { - if (entry.snapshot.getSnapshotId() == null) { - continue; - } builder.startObject(); { builder.field("repository", entry.snapshot.getRepository()); @@ -157,11 +153,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public String toString() { StringBuilder builder = new StringBuilder("SnapshotDeletionsInProgress["); for (int i = 0; i < entries.size(); i++) { - final SnapshotId snapshotId = entries.get(i).getSnapshot().getSnapshotId(); - if (snapshotId == null) { - continue; - } - builder.append(snapshotId.getName()); + builder.append(entries.get(i).getSnapshot().getSnapshotId().getName()); if (i + 1 < entries.size()) { builder.append(","); } diff --git a/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java b/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java index d55988e5d449d..2847af386b2e1 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java +++ b/server/src/main/java/org/elasticsearch/snapshots/Snapshot.java @@ -19,8 +19,6 @@ package org.elasticsearch.snapshots; -import org.elasticsearch.Version; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -40,9 +38,9 @@ public final class Snapshot implements Writeable { /** * Constructs a snapshot. */ - public Snapshot(String repository, @Nullable SnapshotId snapshotId) { + public Snapshot(final String repository, final SnapshotId snapshotId) { this.repository = Objects.requireNonNull(repository); - this.snapshotId = snapshotId; + this.snapshotId = Objects.requireNonNull(snapshotId); this.hashCode = computeHashCode(); } @@ -51,11 +49,7 @@ public Snapshot(String repository, @Nullable SnapshotId snapshotId) { */ public Snapshot(final StreamInput in) throws IOException { repository = in.readString(); - if (in.getVersion().onOrAfter(Version.V_8_0_0)) { - snapshotId = in.readOptionalWriteable(SnapshotId::new); - } else { - snapshotId = new SnapshotId(in); - } + snapshotId = new SnapshotId(in); hashCode = computeHashCode(); } @@ -69,7 +63,6 @@ public String getRepository() { /** * Gets the snapshot id for the snapshot. */ - @Nullable public SnapshotId getSnapshotId() { return snapshotId; } @@ -88,7 +81,7 @@ public boolean equals(Object o) { return false; } Snapshot that = (Snapshot) o; - return repository.equals(that.repository) && Objects.equals(snapshotId, that.snapshotId); + return repository.equals(that.repository) && snapshotId.equals(that.snapshotId); } @Override @@ -103,11 +96,7 @@ private int computeHashCode() { @Override public void writeTo(final StreamOutput out) throws IOException { out.writeString(repository); - if (out.getVersion().onOrAfter(Version.V_8_0_0)) { - out.writeOptionalWriteable(snapshotId); - } else { - snapshotId.writeTo(out); - } + snapshotId.writeTo(out); } } diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 6dfe2290e768d..138c7a88062e4 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -33,6 +33,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateApplier; import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.RepositoryCleanupInProgress; import org.elasticsearch.cluster.RestoreInProgress; import org.elasticsearch.cluster.SnapshotDeletionsInProgress; import org.elasticsearch.cluster.SnapshotsInProgress; @@ -266,6 +267,11 @@ public ClusterState execute(ClusterState currentState) { throw new ConcurrentSnapshotExecutionException(repositoryName, snapshotName, "cannot snapshot while a snapshot deletion is in-progress"); } + final RepositoryCleanupInProgress repositoryCleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); + if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.isSafe(repositoryName) == false) { + throw new ConcurrentSnapshotExecutionException(repositoryName, snapshotName, + "cannot snapshot while a repository cleanup is in-progress"); + } SnapshotsInProgress snapshots = currentState.custom(SnapshotsInProgress.TYPE); if (snapshots == null || snapshots.entries().isEmpty()) { // Store newSnapshot here to be processed in clusterStateProcessed @@ -1130,14 +1136,19 @@ public void cleanupRepo(String repositoryName, ActionListener action.path.startsWith(thisPath)) - .forEach(a -> context.actions.add(new BlobStoreAction(Operation.DELETE, a.path))); + .forEach(a -> { + context.actions.add(new BlobStoreAction(Operation.DELETE, a.path)); + resultConsumer.accept(a.data.length); + }); } } From d4ae1c0b464c22f09d02eed38d295e41ef2561e8 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 5 Aug 2019 12:20:57 +0200 Subject: [PATCH 45/66] resolve conflict --- .../java/org/elasticsearch/snapshots/AbstractCleanupTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/AbstractCleanupTests.java b/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/AbstractCleanupTests.java index 3d3c9a9e9ef9e..3bdc18864debe 100644 --- a/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/AbstractCleanupTests.java +++ b/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/AbstractCleanupTests.java @@ -63,7 +63,7 @@ private void cleanupRepository(BlobStoreRepository repository) throws Exception repository.threadPool().generic().execute(new ActionRunnable<>(future) { @Override protected void doRun() throws Exception { - repository.blobStore().blobContainer(repository.basePath()).delete(); + repository.blobStore().blobContainer(repository.basePath()).delete(ignored -> {}); future.onResponse(null); } }); From 461c732f80f3b21f941fab459fcef9e8f6a9e5fa Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 5 Aug 2019 13:31:10 +0200 Subject: [PATCH 46/66] shorter --- .../cluster/RepositoryCleanupInProgress.java | 19 ++---- .../snapshots/SnapshotsService.java | 60 ++++++++++++++----- 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java b/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java index 8c70a75b2c06e..4a6e6df3a48e4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java @@ -39,7 +39,7 @@ public RepositoryCleanupInProgress(Entry... entries) { this.entries = Arrays.asList(entries); } - public RepositoryCleanupInProgress(StreamInput in) throws IOException { + RepositoryCleanupInProgress(StreamInput in) throws IOException { this.entries = in.readList(Entry::new); } @@ -48,17 +48,12 @@ public static NamedDiff readDiffFrom(StreamInput in) throws } public static Entry startedEntry(String repository, long repositoryStateId) { - return new Entry(repository, repositoryStateId, false); + return new Entry(repository, repositoryStateId); } - public static Entry safeEntry(String repository, long repositoryStateId) { - return new Entry(repository, repositoryStateId, true); - } - - public boolean isSafe(String repository) { + public boolean cleanupInProgress() { // TODO: Should we allow parallelism across repositories here maybe? - assert repository != null; - return entries.stream().noneMatch(entry -> entry.safe == false); + return entries.isEmpty(); } @Override @@ -101,18 +96,14 @@ public static final class Entry implements Writeable { private final long repositoryStateId; - private final boolean safe; - private Entry(StreamInput in) throws IOException { repository = in.readString(); repositoryStateId = in.readLong(); - safe = in.readBoolean(); } - private Entry(String repository, long repositoryStateId, boolean safe) { + private Entry(String repository, long repositoryStateId) { this.repository = repository; this.repositoryStateId = repositoryStateId; - this.safe = safe; } @Override diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 68cb25ad1c9a1..3af8254f24a7a 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -268,7 +268,7 @@ public ClusterState execute(ClusterState currentState) { "cannot snapshot while a snapshot deletion is in-progress"); } final RepositoryCleanupInProgress repositoryCleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); - if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.isSafe(repositoryName) == false) { + if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.cleanupInProgress() == false) { throw new ConcurrentSnapshotExecutionException(repositoryName, snapshotName, "cannot snapshot while a repository cleanup is in-progress"); } @@ -1131,13 +1131,12 @@ public void cleanupRepo(String repositoryName, ActionListener l.onResponse(result))); + assert e != null || result != null; + clusterService.submitStateUpdateTask("Remove repository cleanup task", new ClusterStateUpdateTask() { + @Override + public ClusterState execute(ClusterState currentState) { + RepositoryCleanupInProgress cleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); + if (cleanupInProgress != null) { + boolean changed = false; + if (cleanupInProgress.cleanupInProgress() == false) { + cleanupInProgress = new RepositoryCleanupInProgress(); + changed = true; + } + if (changed) { + return ClusterState.builder(currentState).putCustom( + RepositoryCleanupInProgress.TYPE, cleanupInProgress).build(); + } + } + return currentState; + } + + @Override + public void onFailure(String source, Exception e) { + logger.warn(() -> new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); + if (listener != null) { + listener.onFailure(e); + } + } + + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + if (listener != null) { + if (e == null) { + listener.onResponse(result); + } else { + listener.onFailure(e); + } + } + } + }); } }); } @@ -1212,7 +1240,7 @@ public ClusterState execute(ClusterState currentState) { "cannot delete - another snapshot is currently being deleted"); } final RepositoryCleanupInProgress repositoryCleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); - if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.isSafe(snapshot.getRepository()) == false) { + if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.cleanupInProgress() == false) { throw new ConcurrentSnapshotExecutionException(snapshot.getRepository(), snapshot.getSnapshotId().getName(), "cannot delete snapshot while a repository cleanup is in-progress"); } From 10d158faa89be304a35f585d09642a8197e95169 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 5 Aug 2019 13:40:05 +0200 Subject: [PATCH 47/66] nicer --- .../TransportCleanupRepositoryAction.java | 117 +++++++++++++++++- .../snapshots/SnapshotsService.java | 99 --------------- 2 files changed, 112 insertions(+), 104 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index cfb1bb0a98ccd..a11c7e8d1d515 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -18,18 +18,28 @@ */ package org.elasticsearch.action.admin.cluster.repositories.cleanup; +import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRunnable; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.TransportMasterNodeAction; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.RepositoryCleanupInProgress; +import org.elasticsearch.cluster.SnapshotDeletionsInProgress; +import org.elasticsearch.cluster.SnapshotsInProgress; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.snapshots.SnapshotsService; +import org.elasticsearch.repositories.RepositoriesService; +import org.elasticsearch.repositories.Repository; +import org.elasticsearch.repositories.RepositoryCleanupResult; +import org.elasticsearch.repositories.blobstore.BlobStoreRepository; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -41,7 +51,7 @@ public final class TransportCleanupRepositoryAction extends TransportMasterNodeA private static final Version MIN_VERSION = Version.V_8_0_0; - private final SnapshotsService snapshotsService; + private final RepositoriesService repositoriesService; @Override protected String executor() { @@ -50,11 +60,11 @@ protected String executor() { @Inject public TransportCleanupRepositoryAction(TransportService transportService, ClusterService clusterService, - SnapshotsService snapshotsService, ThreadPool threadPool, ActionFilters actionFilters, + RepositoriesService repositoriesService, ThreadPool threadPool, ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) { super(CleanupRepositoryAction.NAME, transportService, clusterService, threadPool, actionFilters, CleanupRepositoryRequest::new, indexNameExpressionResolver); - this.snapshotsService = snapshotsService; + this.repositoriesService = repositoriesService; } @Override @@ -66,7 +76,7 @@ protected CleanupRepositoryResponse read(StreamInput in) throws IOException { protected void masterOperation(Task task, CleanupRepositoryRequest request, ClusterState state, ActionListener listener) { if (state.nodes().getMinNodeVersion().onOrAfter(MIN_VERSION)) { - snapshotsService.cleanupRepo(request.repository(), ActionListener.map(listener, CleanupRepositoryResponse::new)); + cleanupRepo(request.repository(), ActionListener.map(listener, CleanupRepositoryResponse::new)); } else { throw new IllegalArgumentException("Repository cleanup is only supported from version [" + MIN_VERSION + "] but the oldest node version in the cluster is [" + state.nodes().getMinNodeVersion() + ']'); @@ -78,4 +88,101 @@ protected ClusterBlockException checkBlock(CleanupRepositoryRequest request, Clu // Cluster is not affected but we look up repositories in metadata return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ); } + + + /** + * Runs cleanup operations on the given repository. + * + * @param repositoryName Repository to clean up + * @param listener Listener for cleanup result + */ + private void cleanupRepo(String repositoryName, ActionListener listener) { + final Repository repository = repositoriesService.repository(repositoryName); + if (repository instanceof BlobStoreRepository == false) { + listener.onFailure(new IllegalArgumentException("Repository [" + repositoryName + "] does not support repository cleanup")); + return; + } + final long repositoryStateId = repository.getRepositoryData().getGenId(); + clusterService.submitStateUpdateTask("cleanup repository", new ClusterStateUpdateTask() { + @Override + public ClusterState execute(ClusterState currentState) { + SnapshotDeletionsInProgress deletionsInProgress = currentState.custom(SnapshotDeletionsInProgress.TYPE); + final RepositoryCleanupInProgress repositoryCleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); + if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.cleanupInProgress() == false) { + throw new IllegalStateException( + "Cannot cleanup [" + repositoryName + "] - a repository cleanup is already in-progress"); + } + if (deletionsInProgress != null && deletionsInProgress.hasDeletionsInProgress()) { + throw new IllegalStateException("Cannot cleanup [" + repositoryName + "] - a snapshot is currently being deleted"); + } + ClusterState.Builder clusterStateBuilder = ClusterState.builder(currentState); + SnapshotsInProgress snapshots = currentState.custom(SnapshotsInProgress.TYPE); + if (snapshots != null && !snapshots.entries().isEmpty()) { + throw new IllegalStateException("Cannot cleanup [" + repositoryName + "] - a snapshot is currently running"); + } + final RepositoryCleanupInProgress newCleanupInProgress = + new RepositoryCleanupInProgress(RepositoryCleanupInProgress.startedEntry(repositoryName, repositoryStateId)); + return clusterStateBuilder.putCustom(RepositoryCleanupInProgress.TYPE, newCleanupInProgress).build(); + } + + @Override + public void onFailure(String source, Exception e) { + after(e, null); + } + + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(new ActionRunnable<>(listener) { + @Override + protected void doRun() { + Repository repository = repositoriesService.repository(repositoryName); + assert repository instanceof BlobStoreRepository; + ((BlobStoreRepository) repository) + .cleanup(repositoryStateId, ActionListener.wrap(result -> after(null, result), e -> after(e, null))); + } + }); + } + + private void after(@Nullable Exception e, @Nullable RepositoryCleanupResult result) { + assert e != null || result != null; + clusterService.submitStateUpdateTask("Remove repository cleanup task", new ClusterStateUpdateTask() { + @Override + public ClusterState execute(ClusterState currentState) { + RepositoryCleanupInProgress cleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); + if (cleanupInProgress != null) { + boolean changed = false; + if (cleanupInProgress.cleanupInProgress() == false) { + cleanupInProgress = new RepositoryCleanupInProgress(); + changed = true; + } + if (changed) { + return ClusterState.builder(currentState).putCustom( + RepositoryCleanupInProgress.TYPE, cleanupInProgress).build(); + } + } + return currentState; + } + + @Override + public void onFailure(String source, Exception e) { + logger.warn(() -> new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); + if (listener != null) { + listener.onFailure(e); + } + } + + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + if (listener != null) { + if (e == null) { + listener.onResponse(result); + } else { + listener.onFailure(e); + } + } + } + }); + } + }); + } } diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 3af8254f24a7a..7364f1e859de2 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -67,10 +67,8 @@ import org.elasticsearch.repositories.IndexId; import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.repositories.Repository; -import org.elasticsearch.repositories.RepositoryCleanupResult; import org.elasticsearch.repositories.RepositoryData; import org.elasticsearch.repositories.RepositoryMissingException; -import org.elasticsearch.repositories.blobstore.BlobStoreRepository; import org.elasticsearch.threadpool.ThreadPool; import java.io.IOException; @@ -1118,103 +1116,6 @@ public void deleteSnapshot(final String repositoryName, final String snapshotNam deleteSnapshot(new Snapshot(repositoryName, matchedEntry.get()), listener, repoGenId, immediatePriority); } - /** - * Runs cleanup operations on the given repository. - * - * @param repositoryName Repository to clean up - * @param listener Listener for cleanup result - */ - public void cleanupRepo(String repositoryName, ActionListener listener) { - final Repository repository = repositoriesService.repository(repositoryName); - if (repository instanceof BlobStoreRepository == false) { - listener.onFailure(new IllegalArgumentException("Repository [" + repositoryName + "] does not support repository cleanup")); - return; - } - final long repositoryStateId = repository.getRepositoryData().getGenId(); - clusterService.submitStateUpdateTask("cleanup repository", new ClusterStateUpdateTask() { - @Override - public ClusterState execute(ClusterState currentState) { - SnapshotDeletionsInProgress deletionsInProgress = currentState.custom(SnapshotDeletionsInProgress.TYPE); - final RepositoryCleanupInProgress repositoryCleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); - if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.cleanupInProgress() == false) { - throw new IllegalStateException( - "Cannot cleanup [" + repositoryName + "] - a repository cleanup is already in-progress"); - } - if (deletionsInProgress != null && deletionsInProgress.hasDeletionsInProgress()) { - throw new IllegalStateException("Cannot cleanup [" + repositoryName + "] - a snapshot is currently being deleted"); - } - ClusterState.Builder clusterStateBuilder = ClusterState.builder(currentState); - SnapshotsInProgress snapshots = currentState.custom(SnapshotsInProgress.TYPE); - if (snapshots != null && !snapshots.entries().isEmpty()) { - throw new IllegalStateException("Cannot cleanup [" + repositoryName + "] - a snapshot is currently running"); - } - final RepositoryCleanupInProgress newCleanupInProgress = - new RepositoryCleanupInProgress(RepositoryCleanupInProgress.startedEntry(repositoryName, repositoryStateId)); - return clusterStateBuilder.putCustom(RepositoryCleanupInProgress.TYPE, newCleanupInProgress).build(); - } - - @Override - public void onFailure(String source, Exception e) { - after(e, null); - } - - @Override - public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { - threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(new ActionRunnable<>(listener) { - @Override - protected void doRun() { - Repository repository = repositoriesService.repository(repositoryName); - assert repository instanceof BlobStoreRepository; - ((BlobStoreRepository) repository) - .cleanup(repositoryStateId, ActionListener.wrap(result -> after(null, result), e -> after(e, null))); - } - }); - } - - private void after(@Nullable Exception e, @Nullable RepositoryCleanupResult result) { - assert e != null || result != null; - clusterService.submitStateUpdateTask("Remove repository cleanup task", new ClusterStateUpdateTask() { - @Override - public ClusterState execute(ClusterState currentState) { - RepositoryCleanupInProgress cleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); - if (cleanupInProgress != null) { - boolean changed = false; - if (cleanupInProgress.cleanupInProgress() == false) { - cleanupInProgress = new RepositoryCleanupInProgress(); - changed = true; - } - if (changed) { - return ClusterState.builder(currentState).putCustom( - RepositoryCleanupInProgress.TYPE, cleanupInProgress).build(); - } - } - return currentState; - } - - @Override - public void onFailure(String source, Exception e) { - logger.warn(() -> new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); - if (listener != null) { - listener.onFailure(e); - } - } - - @Override - public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { - if (listener != null) { - if (e == null) { - listener.onResponse(result); - } else { - listener.onFailure(e); - } - } - } - }); - } - }); - } - - /** * Deletes snapshot from repository. *

From 2e2a52f8961db6004a24c49e92b3d435293f143f Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 5 Aug 2019 14:06:40 +0200 Subject: [PATCH 48/66] nicer --- .../TransportCleanupRepositoryAction.java | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index a11c7e8d1d515..688bb69044ca9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -18,6 +18,8 @@ */ package org.elasticsearch.action.admin.cluster.repositories.cleanup; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; @@ -47,7 +49,9 @@ import java.io.IOException; public final class TransportCleanupRepositoryAction extends TransportMasterNodeAction { + CleanupRepositoryResponse> { + + private static final Logger logger = LogManager.getLogger(TransportCleanupRepositoryAction.class); private static final Version MIN_VERSION = Version.V_8_0_0; @@ -74,7 +78,7 @@ protected CleanupRepositoryResponse read(StreamInput in) throws IOException { @Override protected void masterOperation(Task task, CleanupRepositoryRequest request, ClusterState state, - ActionListener listener) { + ActionListener listener) { if (state.nodes().getMinNodeVersion().onOrAfter(MIN_VERSION)) { cleanupRepo(request.repository(), ActionListener.map(listener, CleanupRepositoryResponse::new)); } else { @@ -89,10 +93,8 @@ protected ClusterBlockException checkBlock(CleanupRepositoryRequest request, Clu return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ); } - /** * Runs cleanup operations on the given repository. - * * @param repositoryName Repository to clean up * @param listener Listener for cleanup result */ @@ -143,8 +145,8 @@ protected void doRun() { }); } - private void after(@Nullable Exception e, @Nullable RepositoryCleanupResult result) { - assert e != null || result != null; + private void after(@Nullable Exception failure, @Nullable RepositoryCleanupResult result) { + assert failure != null || result != null; clusterService.submitStateUpdateTask("Remove repository cleanup task", new ClusterStateUpdateTask() { @Override public ClusterState execute(ClusterState currentState) { @@ -166,19 +168,18 @@ public ClusterState execute(ClusterState currentState) { @Override public void onFailure(String source, Exception e) { logger.warn(() -> new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); - if (listener != null) { - listener.onFailure(e); + if (failure != null) { + e.addSuppressed(failure); } + listener.onFailure(e); } @Override public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { - if (listener != null) { - if (e == null) { - listener.onResponse(result); - } else { - listener.onFailure(e); - } + if (failure == null) { + listener.onResponse(result); + } else { + listener.onFailure(failure); } } }); From 776b75160527340d6be62d619502f4671d5b9b47 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 5 Aug 2019 19:31:23 +0200 Subject: [PATCH 49/66] docs + correct handling read-only repo --- .../TransportCleanupRepositoryAction.java | 78 +++++++++++++++---- .../cluster/RepositoryCleanupInProgress.java | 5 ++ .../blobstore/BlobStoreRepository.java | 19 +++-- .../AbstractSnapshotIntegTestCase.java | 13 ++-- 4 files changed, 87 insertions(+), 28 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 688bb69044ca9..0800f32fe0796 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -48,8 +48,25 @@ import java.io.IOException; +/** + * Repository cleanup action for repository implementations based on {@link BlobStoreRepository}. + * + * The steps taken by the repository cleanup operation are as follows: + *

    + *
  1. Check that there are no running repository cleanup, snapshot create, or snapshot delete actions + * and add an entry for the repository that is to be cleaned up to {@link RepositoryCleanupInProgress}
    2. + *
  2. Run cleanup actions on the repository. Note, these are executed exclusively on the master node. + * For the precise operations execute see {@link BlobStoreRepository#cleanup}
    3. + *
  3. Remove the entry in {@link RepositoryCleanupInProgress} in the first step.
    4. + *
+ * + * On master failover during the cleanup operation it is simply removed from the cluster state. This is safe because the logic in + * {@link BlobStoreRepository#cleanup} ensures that the repository state id has not changed between creation of the cluster state entry + * and any delete/write operations. TODO: This will not work if we also want to clean up at the shard level as those will involve writes + * as well as deletes. + */ public final class TransportCleanupRepositoryAction extends TransportMasterNodeAction { + CleanupRepositoryResponse> { private static final Logger logger = LogManager.getLogger(TransportCleanupRepositoryAction.class); @@ -69,6 +86,51 @@ public TransportCleanupRepositoryAction(TransportService transportService, Clust super(CleanupRepositoryAction.NAME, transportService, clusterService, threadPool, actionFilters, CleanupRepositoryRequest::new, indexNameExpressionResolver); this.repositoriesService = repositoriesService; + // We add a state applier that will remove any dangling repository cleanup actions on master failover. + // This is safe to do since cleanups will increment the repository state id before executing any operations to prevent concurrent + // operations from corrupting the repository. This is the same safety mechanism used by snapshot deletes. + clusterService.addStateApplier(event -> { + if (event.localNodeMaster() && event.previousState().nodes().isLocalNodeElectedMaster() == false) { + final RepositoryCleanupInProgress repositoryCleanupInProgress = event.state().custom(RepositoryCleanupInProgress.TYPE); + if (repositoryCleanupInProgress == null || repositoryCleanupInProgress.cleanupInProgress() == false) { + return; + } + clusterService.submitStateUpdateTask("Clean up repository cleanup task after master failover", + new ClusterStateUpdateTask() { + @Override + public ClusterState execute(ClusterState currentState) { + return removeInProgressCleanup(currentState); + } + + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + logger.debug("Removed repository cleanup task [{}] from cluster state", repositoryCleanupInProgress); + } + + @Override + public void onFailure(String source, Exception e) { + logger.warn( + "Failed to remove repository cleanup task [{}] from cluster state", repositoryCleanupInProgress); + } + }); + } + }); + } + + private static ClusterState removeInProgressCleanup(final ClusterState currentState) { + RepositoryCleanupInProgress cleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); + if (cleanupInProgress != null) { + boolean changed = false; + if (cleanupInProgress.cleanupInProgress() == false) { + cleanupInProgress = new RepositoryCleanupInProgress(); + changed = true; + } + if (changed) { + return ClusterState.builder(currentState).putCustom( + RepositoryCleanupInProgress.TYPE, cleanupInProgress).build(); + } + } + return currentState; } @Override @@ -150,19 +212,7 @@ private void after(@Nullable Exception failure, @Nullable RepositoryCleanupResul clusterService.submitStateUpdateTask("Remove repository cleanup task", new ClusterStateUpdateTask() { @Override public ClusterState execute(ClusterState currentState) { - RepositoryCleanupInProgress cleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); - if (cleanupInProgress != null) { - boolean changed = false; - if (cleanupInProgress.cleanupInProgress() == false) { - cleanupInProgress = new RepositoryCleanupInProgress(); - changed = true; - } - if (changed) { - return ClusterState.builder(currentState).putCustom( - RepositoryCleanupInProgress.TYPE, cleanupInProgress).build(); - } - } - return currentState; + return removeInProgressCleanup(currentState); } @Override diff --git a/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java b/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java index 4a6e6df3a48e4..9dfd5284fd8c7 100644 --- a/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/RepositoryCleanupInProgress.java @@ -111,5 +111,10 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(repository); out.writeLong(repositoryStateId); } + + @Override + public String toString() { + return "{" + repository + '}' + '{' + repositoryStateId + '}'; + } } } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 61faebc92c661..345131c56e78d 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -437,7 +437,8 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action } /** - * Runs all cleanup actions on the repository. + * Runs cleanup actions on the repository. Increments the repository state id by one before executing any modifications on the + * repository. * TODO: Add shard level cleanups *
    *
  • Deleting stale indices {@link #cleanupStaleIndices}
    • @@ -448,18 +449,22 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action */ public void cleanup(long repositoryStateId, ActionListener listener) { ActionListener.completeWith(listener, () -> { + if (isReadOnly()) { + throw new RepositoryException(metadata.name(), "cannot run cleanup on readonly repository"); + } final RepositoryData repositoryData = getRepositoryData(); if (repositoryData.getGenId() != repositoryStateId) { - // the index file was updated by a concurrent operation, so we were operating on stale - // repository data - throw new RepositoryException(metadata.name(), "concurrent modification of the repository, expected current generation [" + - repositoryStateId + "], actual current generation [" + repositoryData.getGenId() + - "] - possibly due to simultaneous snapshot deletion or creation requests"); + // Check that we are working on the expected repository version before gathering the data to clean up + throw new RepositoryException(metadata.name(), "concurrent modification of the repository before cleanup started, " + + "expected current generation [" + repositoryStateId + "], actual current generation [" + + repositoryData.getGenId() + "]"); } + Map rootBlobs = blobContainer().listBlobs(); final Map foundIndices = blobStore().blobContainer(indicesPath()).children(); + // write new index-N blob to ensure concurrent operations will fail + writeIndexGen(repositoryData, repositoryStateId); final RepositoryCleanupResult.Progress progress = RepositoryCleanupResult.start(); cleanupStaleIndices(foundIndices, repositoryData.getIndices(), progress); - Map rootBlobs = blobContainer().listBlobs(); List cleaned = cleanupStaleRootFiles(rootBlobs.keySet(), repositoryData); for (String name : cleaned) { progress.accept(rootBlobs.get(name).length()); diff --git a/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java b/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java index ebe6caaf8fd97..9766663d58b77 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java +++ b/server/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java @@ -20,7 +20,6 @@ import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; import org.elasticsearch.cluster.SnapshotsInProgress; -import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; @@ -72,13 +71,13 @@ public void assertConsistentHistoryInLuceneIndex() throws Exception { @After public void assertRepoConsistency() { if (skipRepoConsistencyCheckReason == null) { - client().admin().cluster().prepareGetRepositories().get().repositories() - .stream() - .map(RepositoryMetaData::name) - .forEach(name -> { + client().admin().cluster().prepareGetRepositories().get().repositories().forEach(repositoryMetaData -> { + final String name = repositoryMetaData.name(); + if (repositoryMetaData.settings().getAsBoolean("readonly", false) == false) { client().admin().cluster().prepareCleanupRepository(name).get(); - BlobStoreTestUtil.assertRepoConsistency(internalCluster(), name); - }); + } + BlobStoreTestUtil.assertRepoConsistency(internalCluster(), name); + }); } else { logger.info("--> skipped repo consistency checks because [{}]", skipRepoConsistencyCheckReason); } From e9ae50ada46e0e322f7510e8bbaa17353bba19c7 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 5 Aug 2019 19:59:11 +0200 Subject: [PATCH 50/66] More efficient, don't increment index unless there is something to do --- .../blobstore/BlobStoreRepository.java | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 345131c56e78d..1147aa58a34f7 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -428,8 +428,10 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action .orElse(Collections.emptyList()), snapshotId, ActionListener.map(listener, v -> { - cleanupStaleIndices(foundIndices, survivingIndices, l -> {}); - cleanupStaleRootFiles(Sets.difference(rootBlobs, new HashSet<>(snapMetaFilesToDelete)), updatedRepositoryData); + cleanupStaleIndices(foundIndices, survivingIndices.values().stream() + .map(IndexId::getId).collect(Collectors.toSet()), l -> {}); + cleanupStaleRootFiles( + staleRootBlobs(updatedRepositoryData, Sets.difference(rootBlobs, new HashSet<>(snapMetaFilesToDelete)))); return null; }) ); @@ -461,11 +463,18 @@ public void cleanup(long repositoryStateId, ActionListener rootBlobs = blobContainer().listBlobs(); final Map foundIndices = blobStore().blobContainer(indicesPath()).children(); + final Set survivingIndexIds = + repositoryData.getIndices().values().stream().map(IndexId::getId).collect(Collectors.toSet()); + final RepositoryCleanupResult.Progress progress = RepositoryCleanupResult.start(); + final List staleRootBlobs = staleRootBlobs(repositoryData, rootBlobs.keySet()); + if (survivingIndexIds.equals(foundIndices.keySet()) && staleRootBlobs.isEmpty()) { + // Nothing to clean up we return + return progress.finish(); + } // write new index-N blob to ensure concurrent operations will fail writeIndexGen(repositoryData, repositoryStateId); - final RepositoryCleanupResult.Progress progress = RepositoryCleanupResult.start(); - cleanupStaleIndices(foundIndices, repositoryData.getIndices(), progress); - List cleaned = cleanupStaleRootFiles(rootBlobs.keySet(), repositoryData); + cleanupStaleIndices(foundIndices, survivingIndexIds, progress); + List cleaned = cleanupStaleRootFiles(staleRootBlobs); for (String name : cleaned) { progress.accept(rootBlobs.get(name).length()); } @@ -473,10 +482,10 @@ public void cleanup(long repositoryStateId, ActionListener cleanupStaleRootFiles(Set rootBlobNames, RepositoryData repositoryData) { + private List staleRootBlobs(RepositoryData repositoryData, Set rootBlobNames) { final Set allSnapshotIds = repositoryData.getSnapshotIds().stream().map(SnapshotId::getUUID).collect(Collectors.toSet()); - final List blobsToDelete = rootBlobNames.stream().filter( + return rootBlobNames.stream().filter( blob -> { if (FsBlobContainer.isTempBlobName(blob)) { return true; @@ -497,6 +506,9 @@ private List cleanupStaleRootFiles(Set rootBlobNames, Repository return false; } ).collect(Collectors.toList()); + } + + private List cleanupStaleRootFiles(List blobsToDelete) { if (blobsToDelete.isEmpty()) { return blobsToDelete; } @@ -518,11 +530,9 @@ private List cleanupStaleRootFiles(Set rootBlobNames, Repository return Collections.emptyList(); } - private void cleanupStaleIndices(Map foundIndices, Map survivingIndices, + private void cleanupStaleIndices(Map foundIndices, Set survivingIndexIds, LongConsumer progress) { try { - final Set survivingIndexIds = survivingIndices.values().stream() - .map(IndexId::getId).collect(Collectors.toSet()); for (Map.Entry indexEntry : foundIndices.entrySet()) { final String indexSnId = indexEntry.getKey(); try { From 93d395ce0a38f9f7bfb6e428c8fc87b1a9a7690a Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 5 Aug 2019 19:59:58 +0200 Subject: [PATCH 51/66] nicer formatting --- .../repositories/blobstore/BlobStoreRepository.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 1147aa58a34f7..3a6bb4873b706 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -530,8 +530,7 @@ private List cleanupStaleRootFiles(List blobsToDelete) { return Collections.emptyList(); } - private void cleanupStaleIndices(Map foundIndices, Set survivingIndexIds, - LongConsumer progress) { + private void cleanupStaleIndices(Map foundIndices, Set survivingIndexIds, LongConsumer progress) { try { for (Map.Entry indexEntry : foundIndices.entrySet()) { final String indexSnId = indexEntry.getKey(); From 590295b0437ece429648421988ac8c156f66db04 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 5 Aug 2019 20:01:46 +0200 Subject: [PATCH 52/66] add comment --- .../repositories/blobstore/BlobStoreRepository.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 3a6bb4873b706..d9205643aca49 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -482,6 +482,7 @@ public void cleanup(long repositoryStateId, ActionListener staleRootBlobs(RepositoryData repositoryData, Set rootBlobNames) { final Set allSnapshotIds = repositoryData.getSnapshotIds().stream().map(SnapshotId::getUUID).collect(Collectors.toSet()); From c28843235d75c6b2ab4ab0bc16f6cf56baa4db54 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 6 Aug 2019 08:02:37 +0200 Subject: [PATCH 53/66] add some logging --- .../cleanup/TransportCleanupRepositoryAction.java | 14 +++++++++++++- .../repositories/RepositoryCleanupResult.java | 6 ++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 0800f32fe0796..8482791401530 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -167,6 +167,7 @@ private void cleanupRepo(String repositoryName, ActionListener(listener) { @Override protected void doRun() { @@ -208,6 +210,12 @@ protected void doRun() { } private void after(@Nullable Exception failure, @Nullable RepositoryCleanupResult result) { + if (failure == null) { + logger.debug("Finished repository cleanup operations on [{}][{}]", repositoryName, repositoryStateId); + } else { + logger.debug(() -> new ParameterizedMessage( + "Failed to finish repository cleanup operations on [{}][{}]", repositoryName, repositoryStateId), failure); + } assert failure != null || result != null; clusterService.submitStateUpdateTask("Remove repository cleanup task", new ClusterStateUpdateTask() { @Override @@ -217,18 +225,22 @@ public ClusterState execute(ClusterState currentState) { @Override public void onFailure(String source, Exception e) { - logger.warn(() -> new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); if (failure != null) { e.addSuppressed(failure); } + logger.warn(() -> new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); listener.onFailure(e); } @Override public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { if (failure == null) { + logger.info( + "Done with repository cleanup on [{}][{}] with result [{}]", repositoryName, repositoryStateId, result); listener.onResponse(result); } else { + logger.warn(() -> new ParameterizedMessage("Failed to run repository cleanup operations on [{}][{}]", + repositoryName, repositoryStateId), failure); listener.onFailure(failure); } } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java index 1c3988aafb46f..df548bc1f040e 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java @@ -19,6 +19,7 @@ package org.elasticsearch.repositories; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -80,6 +81,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder.startObject().field("bytes", bytes).field("blobs", blobs).endObject(); } + @Override + public String toString() { + return Strings.toString(this); + } + public static final class Progress implements LongConsumer { private long bytesCounter; From d40580f9b74f8a76e3f65cab8ad76a27c482be88 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 6 Aug 2019 13:50:56 +0200 Subject: [PATCH 54/66] CR: resolve some comments --- .../main/java/org/elasticsearch/client/SnapshotClient.java | 2 +- .../org/elasticsearch/client/SnapshotRequestConverters.java | 4 ++-- docs/reference/modules/snapshots.asciidoc | 2 +- .../rest-api-spec/api/snapshot.cleanup_repository.json | 2 +- .../cluster/repositories/cleanup/CleanupRepositoryAction.java | 2 +- .../repositories/cleanup/CleanupRepositoryRequest.java | 4 ++-- .../repositories/cleanup/CleanupRepositoryRequestBuilder.java | 2 +- .../cleanup/TransportCleanupRepositoryAction.java | 2 +- .../action/admin/cluster/RestCleanupRepositoryAction.java | 2 +- 9 files changed, 11 insertions(+), 11 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java index 56344bf55c3db..d3b2ea466f458 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java @@ -196,7 +196,7 @@ public CleanupRepositoryResponse cleanupRepository(CleanupRepositoryRequest clea * @param listener the listener to be notified upon request completion */ public void cleanupRepositoryAsync(CleanupRepositoryRequest cleanupRepositoryRequest, RequestOptions options, - ActionListener listener) { + ActionListener listener) { restHighLevelClient.performRequestAsyncAndParseEntity(cleanupRepositoryRequest, SnapshotRequestConverters::cleanupRepository, options, CleanupRepositoryResponse::fromXContent, listener, emptySet()); } diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotRequestConverters.java index 7829637bffb51..703aa0d672555 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotRequestConverters.java @@ -97,8 +97,8 @@ static Request verifyRepository(VerifyRepositoryRequest verifyRepositoryRequest) static Request cleanupRepository(CleanupRepositoryRequest cleanupRepositoryRequest) { String endpoint = new RequestConverters.EndpointBuilder().addPathPartAsIs("_snapshot") - .addPathPart(cleanupRepositoryRequest.repository()) - .addPathPartAsIs("cleanup") + .addPathPart(cleanupRepositoryRequest.name()) + .addPathPartAsIs("_cleanup") .build(); Request request = new Request(HttpPost.METHOD_NAME, endpoint); diff --git a/docs/reference/modules/snapshots.asciidoc b/docs/reference/modules/snapshots.asciidoc index e894df87909c8..4479e939ad863 100644 --- a/docs/reference/modules/snapshots.asciidoc +++ b/docs/reference/modules/snapshots.asciidoc @@ -339,7 +339,7 @@ trigger a complete accounting of the repositories contents and subsequent deleti [source,js] ----------------------------------- -POST /_snapshot/my_repository/cleanup +POST /_snapshot/my_repository/_cleanup ----------------------------------- // CONSOLE // TEST[continued] diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.cleanup_repository.json b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.cleanup_repository.json index 692aa4d0477d8..79223dd214188 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.cleanup_repository.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.cleanup_repository.json @@ -4,7 +4,7 @@ "stability": "stable", "methods": ["POST"], "url": { - "paths": ["/_snapshot/{repository}/cleanup"], + "paths": ["/_snapshot/{repository}/_cleanup"], "parts": { "repository": { "type": "string", diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java index d5b28b4534aca..af57e6d4f00ff 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryAction.java @@ -23,7 +23,7 @@ public final class CleanupRepositoryAction extends ActionType { public static final CleanupRepositoryAction INSTANCE = new CleanupRepositoryAction(); - public static final String NAME = "cluster:admin/repository/cleanup"; + public static final String NAME = "cluster:admin/repository/_cleanup"; private CleanupRepositoryAction() { super(NAME, CleanupRepositoryResponse::new); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java index 12982976b0959..2186bcd451638 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java @@ -53,11 +53,11 @@ public ActionRequestValidationException validate() { return validationException; } - public String repository() { + public String name() { return repository; } - public void repository(String repository) { + public void setName(String repository) { this.repository = repository; } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java index d202877eee941..b5ff18da2d95c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -32,7 +32,7 @@ public CleanupRepositoryRequestBuilder(ElasticsearchClient client, ActionType listener) { if (state.nodes().getMinNodeVersion().onOrAfter(MIN_VERSION)) { - cleanupRepo(request.repository(), ActionListener.map(listener, CleanupRepositoryResponse::new)); + cleanupRepo(request.name(), ActionListener.map(listener, CleanupRepositoryResponse::new)); } else { throw new IllegalArgumentException("Repository cleanup is only supported from version [" + MIN_VERSION + "] but the oldest node version in the cluster is [" + state.nodes().getMinNodeVersion() + ']'); diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java index fa2e06594e9e3..e13c7e37a9a2c 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java @@ -38,7 +38,7 @@ public class RestCleanupRepositoryAction extends BaseRestHandler { public RestCleanupRepositoryAction(Settings settings, RestController controller) { super(settings); - controller.registerHandler(POST, "/_snapshot/{repository}/cleanup", this); + controller.registerHandler(POST, "/_snapshot/{repository}/_cleanup", this); } @Override From c0ca7cdb1938377de932d2c8c0c8d4d8d4dcd04c Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 6 Aug 2019 13:52:52 +0200 Subject: [PATCH 55/66] CR: renamings --- .../repositories/cleanup/CleanupRepositoryRequestBuilder.java | 2 +- .../rest/action/admin/cluster/RestCleanupRepositoryAction.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java index b5ff18da2d95c..1f31f5c10701a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -31,7 +31,7 @@ public CleanupRepositoryRequestBuilder(ElasticsearchClient client, ActionType Date: Tue, 6 Aug 2019 13:53:13 +0200 Subject: [PATCH 56/66] CR: renamings --- .../cluster/repositories/cleanup/CleanupRepositoryRequest.java | 2 +- .../repositories/cleanup/CleanupRepositoryRequestBuilder.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java index 2186bcd451638..168cdbb496701 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequest.java @@ -57,7 +57,7 @@ public String name() { return repository; } - public void setName(String repository) { + public void name(String repository) { this.repository = repository; } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java index 1f31f5c10701a..2f7e6aefdcc94 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/CleanupRepositoryRequestBuilder.java @@ -32,7 +32,7 @@ public CleanupRepositoryRequestBuilder(ElasticsearchClient client, ActionType Date: Tue, 6 Aug 2019 13:55:35 +0200 Subject: [PATCH 57/66] CR: fix indent --- .../cleanup/TransportCleanupRepositoryAction.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 0a20670de15e5..81fb32fd09ed6 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -81,8 +81,8 @@ protected String executor() { @Inject public TransportCleanupRepositoryAction(TransportService transportService, ClusterService clusterService, - RepositoriesService repositoriesService, ThreadPool threadPool, ActionFilters actionFilters, - IndexNameExpressionResolver indexNameExpressionResolver) { + RepositoriesService repositoriesService, ThreadPool threadPool, ActionFilters actionFilters, + IndexNameExpressionResolver indexNameExpressionResolver) { super(CleanupRepositoryAction.NAME, transportService, clusterService, threadPool, actionFilters, CleanupRepositoryRequest::new, indexNameExpressionResolver); this.repositoriesService = repositoriesService; From aef1735e45524d94f5e19aadcb2bb63d024d9550 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 6 Aug 2019 14:01:56 +0200 Subject: [PATCH 58/66] CR: order things reasonably --- .../cleanup/TransportCleanupRepositoryAction.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 81fb32fd09ed6..3586d21546768 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -95,7 +95,7 @@ public TransportCleanupRepositoryAction(TransportService transportService, Clust if (repositoryCleanupInProgress == null || repositoryCleanupInProgress.cleanupInProgress() == false) { return; } - clusterService.submitStateUpdateTask("Clean up repository cleanup task after master failover", + clusterService.submitStateUpdateTask("clean up repository cleanup task after master failover", new ClusterStateUpdateTask() { @Override public ClusterState execute(ClusterState currentState) { @@ -171,23 +171,21 @@ private void cleanupRepo(String repositoryName, ActionListener Date: Tue, 6 Aug 2019 14:19:46 +0200 Subject: [PATCH 59/66] CR: nicer snapshot task name --- .../TransportCleanupRepositoryAction.java | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 3586d21546768..0fb39f2fae4f6 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -215,34 +215,35 @@ private void after(@Nullable Exception failure, @Nullable RepositoryCleanupResul "Failed to finish repository cleanup operations on [{}][{}]", repositoryName, repositoryStateId), failure); } assert failure != null || result != null; - clusterService.submitStateUpdateTask("Remove repository cleanup task", new ClusterStateUpdateTask() { - @Override - public ClusterState execute(ClusterState currentState) { - return removeInProgressCleanup(currentState); - } + clusterService.submitStateUpdateTask("remove repository cleanup task [" +repositoryName + "][" + repositoryStateId + ']', + new ClusterStateUpdateTask() { + @Override + public ClusterState execute(ClusterState currentState) { + return removeInProgressCleanup(currentState); + } - @Override - public void onFailure(String source, Exception e) { - if (failure != null) { - e.addSuppressed(failure); + @Override + public void onFailure(String source, Exception e) { + if (failure != null) { + e.addSuppressed(failure); + } + logger.warn(() -> new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); + listener.onFailure(e); } - logger.warn(() -> new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); - listener.onFailure(e); - } - @Override - public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { - if (failure == null) { - logger.info( - "Done with repository cleanup on [{}][{}] with result [{}]", repositoryName, repositoryStateId, result); - listener.onResponse(result); - } else { - logger.warn(() -> new ParameterizedMessage("Failed to run repository cleanup operations on [{}][{}]", - repositoryName, repositoryStateId), failure); - listener.onFailure(failure); + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + if (failure == null) { + logger.info( + "Done with repository cleanup on [{}][{}] with result [{}]", repositoryName, repositoryStateId, result); + listener.onResponse(result); + } else { + logger.warn(() -> new ParameterizedMessage("Failed to run repository cleanup operations on [{}][{}]", + repositoryName, repositoryStateId), failure); + listener.onFailure(failure); + } } - } - }); + }); } }); } From c9e3e29274e13aab21db060b0b0ab6f0defa11f9 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 6 Aug 2019 14:27:48 +0200 Subject: [PATCH 60/66] CR: nicer naming cleanup CS update tasks --- .../TransportCleanupRepositoryAction.java | 137 +++++++++--------- 1 file changed, 69 insertions(+), 68 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 0fb39f2fae4f6..cad13a2a6b448 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -168,83 +168,84 @@ private void cleanupRepo(String repositoryName, ActionListener(listener) { - @Override - protected void doRun() { + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + logger.debug("Initialized repository cleanup in cluster state for [{}][{}]", repositoryName, repositoryStateId); + threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(ActionRunnable.wrap(listener, l -> { Repository repository = repositoriesService.repository(repositoryName); assert repository instanceof BlobStoreRepository; - ((BlobStoreRepository) repository) - .cleanup(repositoryStateId, ActionListener.wrap(result -> after(null, result), e -> after(e, null))); - } - }); - } - - private void after(@Nullable Exception failure, @Nullable RepositoryCleanupResult result) { - if (failure == null) { - logger.debug("Finished repository cleanup operations on [{}][{}]", repositoryName, repositoryStateId); - } else { - logger.debug(() -> new ParameterizedMessage( - "Failed to finish repository cleanup operations on [{}][{}]", repositoryName, repositoryStateId), failure); + ((BlobStoreRepository) repository).cleanup( + repositoryStateId, ActionListener.wrap(result -> after(null, result), e -> after(e, null))); + })); } - assert failure != null || result != null; - clusterService.submitStateUpdateTask("remove repository cleanup task [" +repositoryName + "][" + repositoryStateId + ']', - new ClusterStateUpdateTask() { - @Override - public ClusterState execute(ClusterState currentState) { - return removeInProgressCleanup(currentState); - } - @Override - public void onFailure(String source, Exception e) { - if (failure != null) { - e.addSuppressed(failure); + private void after(@Nullable Exception failure, @Nullable RepositoryCleanupResult result) { + if (failure == null) { + logger.debug("Finished repository cleanup operations on [{}][{}]", repositoryName, repositoryStateId); + } else { + logger.debug(() -> new ParameterizedMessage( + "Failed to finish repository cleanup operations on [{}][{}]", repositoryName, repositoryStateId), failure); + } + assert failure != null || result != null; + clusterService.submitStateUpdateTask( + "remove repository cleanup task [" + repositoryName + "][" + repositoryStateId + ']', + new ClusterStateUpdateTask() { + @Override + public ClusterState execute(ClusterState currentState) { + return removeInProgressCleanup(currentState); } - logger.warn(() -> new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); - listener.onFailure(e); - } - @Override - public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { - if (failure == null) { - logger.info( - "Done with repository cleanup on [{}][{}] with result [{}]", repositoryName, repositoryStateId, result); - listener.onResponse(result); - } else { - logger.warn(() -> new ParameterizedMessage("Failed to run repository cleanup operations on [{}][{}]", - repositoryName, repositoryStateId), failure); - listener.onFailure(failure); + @Override + public void onFailure(String source, Exception e) { + if (failure != null) { + e.addSuppressed(failure); + } + logger.warn(() -> + new ParameterizedMessage("[{}] failed to remove repository cleanup task", repositoryName), e); + listener.onFailure(e); } - } - }); - } - }); + + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + if (failure == null) { + logger.info("Done with repository cleanup on [{}][{}] with result [{}]", + repositoryName, repositoryStateId, result); + listener.onResponse(result); + } else { + logger.warn(() -> new ParameterizedMessage("Failed to run repository cleanup operations on [{}][{}]", + repositoryName, repositoryStateId), failure); + listener.onFailure(failure); + } + } + }); + } + }); } } From d777610cd06381eff6ff20251a1f21e448e747bb Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 6 Aug 2019 14:39:53 +0200 Subject: [PATCH 61/66] simplify away some casting --- .../cleanup/TransportCleanupRepositoryAction.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index cad13a2a6b448..f234a9e064ae5 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -166,6 +166,7 @@ private void cleanupRepo(String repositoryName, ActionListener { - Repository repository = repositoriesService.repository(repositoryName); - assert repository instanceof BlobStoreRepository; - ((BlobStoreRepository) repository).cleanup( - repositoryStateId, ActionListener.wrap(result -> after(null, result), e -> after(e, null))); - })); + threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(ActionRunnable.wrap(listener, + l -> blobStoreRepository.cleanup( + repositoryStateId, ActionListener.wrap(result -> after(null, result), e -> after(e, null))))); } private void after(@Nullable Exception failure, @Nullable RepositoryCleanupResult result) { From 091fb70ae6658574bf3447e4be2c585b81a31dee Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 6 Aug 2019 15:02:04 +0200 Subject: [PATCH 62/66] CR: rename response fields --- docs/reference/modules/snapshots.asciidoc | 4 ++-- .../rest-api-spec/test/snapshot.create/10_basic.yml | 12 ++++++------ .../repositories/RepositoryCleanupResult.java | 10 +++++++--- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/docs/reference/modules/snapshots.asciidoc b/docs/reference/modules/snapshots.asciidoc index 4479e939ad863..1727caf0d9e91 100644 --- a/docs/reference/modules/snapshots.asciidoc +++ b/docs/reference/modules/snapshots.asciidoc @@ -350,8 +350,8 @@ The response to a cleanup request looks as follows: -------------------------------------------------- { "results": { - "bytes": 20, - "blobs": 5 + "deleted_bytes": 20, + "deleted_blobs": 5 } } -------------------------------------------------- diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml index 42fe445fdea2a..2a33cfbda63d0 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.create/10_basic.yml @@ -48,8 +48,8 @@ setup: snapshot.cleanup_repository: repository: test_repo_create_1 - - match: { results.bytes: 0 } - - match: { results.blobs: 0 } + - match: { results.deleted_bytes: 0 } + - match: { results.deleted_blobs: 0 } - do: snapshot.create: @@ -66,8 +66,8 @@ setup: snapshot.cleanup_repository: repository: test_repo_create_1 - - match: { results.bytes: 0 } - - match: { results.blobs: 0 } + - match: { results.deleted_bytes: 0 } + - match: { results.deleted_blobs: 0 } - do: snapshot.delete: @@ -80,8 +80,8 @@ setup: snapshot.cleanup_repository: repository: test_repo_create_1 - - match: { results.bytes: 0 } - - match: { results.blobs: 0 } + - match: { results.deleted_bytes: 0 } + - match: { results.deleted_blobs: 0 } --- "Create a snapshot for missing index": diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java index df548bc1f040e..b0315ab42f2ad 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java @@ -35,9 +35,13 @@ public final class RepositoryCleanupResult implements Writeable, ToXContentObjec public static final ObjectParser PARSER = new ObjectParser<>(RepositoryCleanupResult.class.getName(), true, RepositoryCleanupResult::new); + private static final String DELETED_BLOBS = "deleted_blobs"; + + private static final String DELETED_BYTES = "deleted_bytes"; + static { - PARSER.declareLong((result, bytes) -> result.bytes = bytes, new ParseField("bytes")); - PARSER.declareLong((result, blobs) -> result.blobs = blobs, new ParseField("blobs")); + PARSER.declareLong((result, bytes) -> result.bytes = bytes, new ParseField(DELETED_BYTES)); + PARSER.declareLong((result, blobs) -> result.blobs = blobs, new ParseField(DELETED_BLOBS)); } private long bytes; @@ -78,7 +82,7 @@ public void writeTo(StreamOutput out) throws IOException { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return builder.startObject().field("bytes", bytes).field("blobs", blobs).endObject(); + return builder.startObject().field(DELETED_BYTES, bytes).field(DELETED_BLOBS, blobs).endObject(); } @Override From c8d513ab151a24cf620e078a4670ec2d6724d259 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 6 Aug 2019 17:32:37 +0200 Subject: [PATCH 63/66] CR: return a delete result from container deletes --- .../blobstore/url/URLBlobContainer.java | 4 +- .../azure/AzureBlobContainer.java | 6 +-- .../repositories/azure/AzureBlobStore.java | 7 ++- .../azure/AzureStorageService.java | 12 +++-- .../gcs/GoogleCloudStorageBlobContainer.java | 6 +-- .../gcs/GoogleCloudStorageBlobStore.java | 15 +++++-- .../repositories/hdfs/HdfsBlobContainer.java | 10 +++-- .../repositories/s3/S3BlobContainer.java | 11 +++-- .../common/blobstore/BlobContainer.java | 8 ++-- .../common/blobstore/DeleteResult.java | 45 +++++++++++++++++++ .../common/blobstore/fs/FsBlobContainer.java | 11 +++-- .../repositories/RepositoryCleanupResult.java | 30 +------------ .../blobstore/BlobStoreRepository.java | 22 ++++----- .../mockstore/BlobContainerWrapper.java | 6 +-- .../MockEventuallyConsistentRepository.java | 11 +++-- .../snapshots/mockstore/MockRepository.java | 14 ++++-- .../AbstractThirdPartyRepositoryTestCase.java | 2 +- .../snapshots/AbstractCleanupTests.java | 2 +- 18 files changed, 137 insertions(+), 85 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/blobstore/DeleteResult.java diff --git a/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java b/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java index f0e11cff2a4b6..7a74078894c92 100644 --- a/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java +++ b/modules/repository-url/src/main/java/org/elasticsearch/common/blobstore/url/URLBlobContainer.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.support.AbstractBlobContainer; import java.io.BufferedInputStream; @@ -35,7 +36,6 @@ import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.Map; -import java.util.function.LongConsumer; /** * URL blob implementation of {@link org.elasticsearch.common.blobstore.BlobContainer} @@ -98,7 +98,7 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete(LongConsumer resultConsumer) { + public DeleteResult delete() { throw new UnsupportedOperationException("URL repository is read only"); } diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java index 7e1f6bd88dfcb..042f08df48262 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobContainer.java @@ -31,6 +31,7 @@ import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.support.AbstractBlobContainer; import org.elasticsearch.threadpool.ThreadPool; @@ -42,7 +43,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ExecutorService; -import java.util.function.LongConsumer; public class AzureBlobContainer extends AbstractBlobContainer { @@ -127,9 +127,9 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete(LongConsumer resultConsumer) throws IOException { + public DeleteResult delete() throws IOException { try { - blobStore.deleteBlobDirectory(keyPath, threadPool.executor(AzureRepositoryPlugin.REPOSITORY_THREAD_POOL_NAME), resultConsumer); + return blobStore.deleteBlobDirectory(keyPath, threadPool.executor(AzureRepositoryPlugin.REPOSITORY_THREAD_POOL_NAME)); } catch (URISyntaxException | StorageException e) { throw new IOException(e); } diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java index f5639b53f52bc..e4a7e3acb6526 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureBlobStore.java @@ -21,12 +21,12 @@ import com.microsoft.azure.storage.LocationMode; import com.microsoft.azure.storage.StorageException; - import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.repositories.azure.AzureRepository.Repository; import org.elasticsearch.threadpool.ThreadPool; @@ -38,7 +38,6 @@ import java.util.Map; import java.util.concurrent.Executor; import java.util.function.Function; -import java.util.function.LongConsumer; import java.util.stream.Collectors; import static java.util.Collections.emptyMap; @@ -93,9 +92,9 @@ public void deleteBlob(String blob) throws URISyntaxException, StorageException service.deleteBlob(clientName, container, blob); } - public void deleteBlobDirectory(String path, Executor executor, LongConsumer resultConsumer) + public DeleteResult deleteBlobDirectory(String path, Executor executor) throws URISyntaxException, StorageException, IOException { - service.deleteBlobDirectory(clientName, container, path, executor, resultConsumer); + return service.deleteBlobDirectory(clientName, container, path, executor); } public InputStream getInputStream(String blob) throws URISyntaxException, StorageException, IOException { diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java index a49de0be332f1..cc4335956b76d 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageService.java @@ -42,6 +42,7 @@ import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.settings.Settings; @@ -67,13 +68,12 @@ import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicLong; -import java.util.function.LongConsumer; import java.util.function.Supplier; import static java.util.Collections.emptyMap; public class AzureStorageService { - + private static final Logger logger = LogManager.getLogger(AzureStorageService.class); public static final ByteSizeValue MIN_CHUNK_SIZE = new ByteSizeValue(1, ByteSizeUnit.BYTES); @@ -193,13 +193,15 @@ public void deleteBlob(String account, String container, String blob) throws URI }); } - void deleteBlobDirectory(String account, String container, String path, Executor executor, LongConsumer resultConsumer) + DeleteResult deleteBlobDirectory(String account, String container, String path, Executor executor) throws URISyntaxException, StorageException, IOException { final Tuple> client = client(account); final CloudBlobContainer blobContainer = client.v1().getContainerReference(container); final Collection exceptions = Collections.synchronizedList(new ArrayList<>()); final AtomicLong outstanding = new AtomicLong(1L); final PlainActionFuture result = PlainActionFuture.newFuture(); + final AtomicLong blobsDeleted = new AtomicLong(); + final AtomicLong bytesDeleted = new AtomicLong(); SocketAccess.doPrivilegedVoidException(() -> { for (final ListBlobItem blobItem : blobContainer.listBlobs(path, true)) { // uri.getPath is of the form /container/keyPath.* and we want to strip off the /container/ @@ -216,8 +218,9 @@ protected void doRun() throws Exception { len = -1L; } deleteBlob(account, container, blobPath); + blobsDeleted.incrementAndGet(); if (len >= 0) { - resultConsumer.accept(len); + bytesDeleted.addAndGet(len); } } @@ -244,6 +247,7 @@ public void onAfter() { exceptions.forEach(ex::addSuppressed); throw ex; } + return new DeleteResult(blobsDeleted.get(), bytesDeleted.get()); } public InputStream getInputStream(String account, String container, String blob) diff --git a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java index 29bda9214a545..da22750242722 100644 --- a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java +++ b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobContainer.java @@ -22,13 +22,13 @@ import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.support.AbstractBlobContainer; import java.io.IOException; import java.io.InputStream; import java.util.List; import java.util.Map; -import java.util.function.LongConsumer; import java.util.stream.Collectors; class GoogleCloudStorageBlobContainer extends AbstractBlobContainer { @@ -78,8 +78,8 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete(LongConsumer resultConsumer) throws IOException { - blobStore.deleteDirectory(path().buildAsString(), resultConsumer); + public DeleteResult delete() throws IOException { + return blobStore.deleteDirectory(path().buildAsString()); } @Override diff --git a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java index ecf6652154f6c..af82679977c09 100644 --- a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java +++ b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java @@ -37,6 +37,7 @@ import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; import org.elasticsearch.common.blobstore.BlobStoreException; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.core.internal.io.Streams; @@ -55,8 +56,8 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.LongConsumer; import java.util.stream.Collectors; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; @@ -301,18 +302,24 @@ void deleteBlob(String blobName) throws IOException { * * @param pathStr Name of path to delete */ - void deleteDirectory(String pathStr, LongConsumer resultConsumer) throws IOException { - SocketAccess.doPrivilegedVoidIOException(() -> { + DeleteResult deleteDirectory(String pathStr) throws IOException { + return SocketAccess.doPrivilegedIOException(() -> { + DeleteResult deleteResult = DeleteResult.ZERO; Page page = client().get(bucketName).list(BlobListOption.prefix(pathStr)); do { final Collection blobsToDelete = new ArrayList<>(); + final AtomicLong blobsDeleted = new AtomicLong(0L); + final AtomicLong bytesDeleted = new AtomicLong(0L); page.getValues().forEach(b -> { blobsToDelete.add(b.getName()); - resultConsumer.accept(b.getSize()); + blobsDeleted.incrementAndGet(); + bytesDeleted.addAndGet(b.getSize()); }); deleteBlobsIgnoringIfNotExists(blobsToDelete); + deleteResult = deleteResult.add(new DeleteResult(blobsDeleted.get(), bytesDeleted.get())); page = page.getNextPage(); } while (page != null); + return deleteResult; }); } diff --git a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java index 1a0ae652d922c..304906464dcad 100644 --- a/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java +++ b/plugins/repository-hdfs/src/main/java/org/elasticsearch/repositories/hdfs/HdfsBlobContainer.java @@ -28,6 +28,7 @@ import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.fs.FsBlobContainer; import org.elasticsearch.common.blobstore.support.AbstractBlobContainer; import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; @@ -43,7 +44,6 @@ import java.util.EnumSet; import java.util.LinkedHashMap; import java.util.Map; -import java.util.function.LongConsumer; final class HdfsBlobContainer extends AbstractBlobContainer { private final HdfsBlobStore store; @@ -70,11 +70,13 @@ public void deleteBlob(String blobName) throws IOException { } } + // TODO: See if we can get precise result reporting. + private static final DeleteResult DELETE_RESULT = new DeleteResult(1L, 0L); + @Override - public void delete(LongConsumer resultConsumer) throws IOException { - // TODO: See if we can get precise result reporting. - resultConsumer.accept(0); + public DeleteResult delete() throws IOException { store.execute(fileContext -> fileContext.delete(path, true)); + return DELETE_RESULT; } @Override diff --git a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java index 1e76f3913c33a..46910d840cd0f 100644 --- a/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java +++ b/plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java @@ -41,6 +41,7 @@ import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.support.AbstractBlobContainer; import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; import org.elasticsearch.common.collect.Tuple; @@ -53,7 +54,7 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.LongConsumer; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; import java.util.stream.Collectors; @@ -121,7 +122,9 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete(LongConsumer resultConsumer) throws IOException { + public DeleteResult delete() throws IOException { + final AtomicLong deletedBlobs = new AtomicLong(); + final AtomicLong deletedBytes = new AtomicLong(); try (AmazonS3Reference clientReference = blobStore.clientReference()) { ObjectListing prevListing = null; while (true) { @@ -137,7 +140,8 @@ public void delete(LongConsumer resultConsumer) throws IOException { } final List blobsToDelete = new ArrayList<>(); list.getObjectSummaries().forEach(s3ObjectSummary -> { - resultConsumer.accept(s3ObjectSummary.getSize()); + deletedBlobs.incrementAndGet(); + deletedBytes.addAndGet(s3ObjectSummary.getSize()); blobsToDelete.add(s3ObjectSummary.getKey()); }); if (list.isTruncated()) { @@ -153,6 +157,7 @@ public void delete(LongConsumer resultConsumer) throws IOException { } catch (final AmazonClientException e) { throw new IOException("Exception when deleting blob container [" + keyPath + "]", e); } + return new DeleteResult(deletedBlobs.get(), deletedBytes.get()); } @Override diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java index 1ff63ab82c58e..83de4aba8e629 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java @@ -25,7 +25,6 @@ import java.nio.file.NoSuchFileException; import java.util.List; import java.util.Map; -import java.util.function.LongConsumer; /** * An interface for managing a repository of blob entries, where each blob entry is just a named group of bytes. @@ -103,12 +102,11 @@ public interface BlobContainer { /** * Deletes this container and all its contents from the repository. - * @param resultConsumer Long consumer that implementations can invoke with the byte size of each deleted blob when deleting it. - * If invoking the result consumer exactly once per deleted blob is not possible for an implementation it must at - * least invoke the consumer once during the delete process if any data was deleted. + * + * @return delete result * @throws IOException on failure */ - void delete(LongConsumer resultConsumer) throws IOException; + DeleteResult delete() throws IOException; /** * Deletes the blobs with given names. Unlike {@link #deleteBlob(String)} this method will not throw an exception diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/DeleteResult.java b/server/src/main/java/org/elasticsearch/common/blobstore/DeleteResult.java new file mode 100644 index 0000000000000..62ba1a588f420 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/blobstore/DeleteResult.java @@ -0,0 +1,45 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.blobstore; + +public final class DeleteResult { + + public static final DeleteResult ZERO = new DeleteResult(0, 0); + + private final long blobsDeleted; + private final long bytesDeleted; + + public DeleteResult(long blobsDeleted, long bytesDeleted) { + this.blobsDeleted = blobsDeleted; + this.bytesDeleted = bytesDeleted; + } + + public long blobsDeleted() { + return blobsDeleted; + } + + public long bytesDeleted() { + return bytesDeleted; + } + + public DeleteResult add(DeleteResult other) { + return new DeleteResult(blobsDeleted + other.blobsDeleted(), bytesDeleted + other.bytesDeleted()); + } +} diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java b/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java index e568265a9b771..d333691a9bc26 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.support.AbstractBlobContainer; import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; import org.elasticsearch.core.internal.io.IOUtils; @@ -45,7 +46,7 @@ import java.nio.file.attribute.BasicFileAttributes; import java.util.HashMap; import java.util.Map; -import java.util.function.LongConsumer; +import java.util.concurrent.atomic.AtomicLong; import static java.util.Collections.unmodifiableMap; @@ -124,7 +125,9 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx } @Override - public void delete(LongConsumer resultConsumer) throws IOException { + public DeleteResult delete() throws IOException { + final AtomicLong filesDeleted = new AtomicLong(0L); + final AtomicLong bytesDeleted = new AtomicLong(0L); Files.walkFileTree(path, new SimpleFileVisitor<>() { @Override public FileVisitResult postVisitDirectory(Path dir, IOException impossible) throws IOException { @@ -136,10 +139,12 @@ public FileVisitResult postVisitDirectory(Path dir, IOException impossible) thro @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { Files.delete(file); - resultConsumer.accept(attrs.size()); + filesDeleted.incrementAndGet(); + bytesDeleted.addAndGet(attrs.size()); return FileVisitResult.CONTINUE; } }); + return new DeleteResult(filesDeleted.get(), bytesDeleted.get()); } @Override diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java index b0315ab42f2ad..b848cb68b7a15 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java @@ -28,7 +28,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; -import java.util.function.LongConsumer; public final class RepositoryCleanupResult implements Writeable, ToXContentObject { @@ -52,9 +51,9 @@ private RepositoryCleanupResult() { this(0L, 0L); } - private RepositoryCleanupResult(long bytes, long blobs) { - this.bytes = bytes; + public RepositoryCleanupResult(long blobs, long bytes) { this.blobs = blobs; + this.bytes = bytes; } public RepositoryCleanupResult(StreamInput in) throws IOException { @@ -70,10 +69,6 @@ public long blobs() { return blobs; } - public static Progress start() { - return new Progress(); - } - @Override public void writeTo(StreamOutput out) throws IOException { out.writeLong(bytes); @@ -89,25 +84,4 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public String toString() { return Strings.toString(this); } - - public static final class Progress implements LongConsumer { - - private long bytesCounter; - - private long blobsCounter; - - @Override - public void accept(long size) { - synchronized (this) { - ++blobsCounter; - bytesCounter += size; - } - } - - public RepositoryCleanupResult finish() { - synchronized (this) { - return new RepositoryCleanupResult(bytesCounter, blobsCounter); - } - } - } } diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index d9205643aca49..c5311ef3e0f8b 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -44,6 +44,7 @@ import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.fs.FsBlobContainer; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; @@ -108,7 +109,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.function.LongConsumer; import java.util.stream.Collectors; import static org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.FileInfo.canonicalName; @@ -428,8 +428,7 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action .orElse(Collections.emptyList()), snapshotId, ActionListener.map(listener, v -> { - cleanupStaleIndices(foundIndices, survivingIndices.values().stream() - .map(IndexId::getId).collect(Collectors.toSet()), l -> {}); + cleanupStaleIndices(foundIndices, survivingIndices.values().stream().map(IndexId::getId).collect(Collectors.toSet())); cleanupStaleRootFiles( staleRootBlobs(updatedRepositoryData, Sets.difference(rootBlobs, new HashSet<>(snapMetaFilesToDelete)))); return null; @@ -465,20 +464,21 @@ public void cleanup(long repositoryStateId, ActionListener foundIndices = blobStore().blobContainer(indicesPath()).children(); final Set survivingIndexIds = repositoryData.getIndices().values().stream().map(IndexId::getId).collect(Collectors.toSet()); - final RepositoryCleanupResult.Progress progress = RepositoryCleanupResult.start(); final List staleRootBlobs = staleRootBlobs(repositoryData, rootBlobs.keySet()); if (survivingIndexIds.equals(foundIndices.keySet()) && staleRootBlobs.isEmpty()) { // Nothing to clean up we return - return progress.finish(); + return new RepositoryCleanupResult(0L, 0L); } // write new index-N blob to ensure concurrent operations will fail writeIndexGen(repositoryData, repositoryStateId); - cleanupStaleIndices(foundIndices, survivingIndexIds, progress); + final DeleteResult deleteIndicesResult = cleanupStaleIndices(foundIndices, survivingIndexIds); List cleaned = cleanupStaleRootFiles(staleRootBlobs); + long deletedRootBytes = 0L; for (String name : cleaned) { - progress.accept(rootBlobs.get(name).length()); + deletedRootBytes += rootBlobs.get(name).length(); } - return progress.finish(); + return new RepositoryCleanupResult( + deleteIndicesResult.blobsDeleted() + cleaned.size(), deletedRootBytes + deleteIndicesResult.bytesDeleted()); }); } @@ -531,14 +531,15 @@ private List cleanupStaleRootFiles(List blobsToDelete) { return Collections.emptyList(); } - private void cleanupStaleIndices(Map foundIndices, Set survivingIndexIds, LongConsumer progress) { + private DeleteResult cleanupStaleIndices(Map foundIndices, Set survivingIndexIds) { + DeleteResult deleteResult = DeleteResult.ZERO; try { for (Map.Entry indexEntry : foundIndices.entrySet()) { final String indexSnId = indexEntry.getKey(); try { if (survivingIndexIds.contains(indexSnId) == false) { logger.debug("[{}] Found stale index [{}]. Cleaning it up", metadata.name(), indexSnId); - indexEntry.getValue().delete(progress); + deleteResult = deleteResult.add(indexEntry.getValue().delete()); logger.debug("[{}] Cleaned up stale index [{}]", metadata.name(), indexSnId); } } catch (IOException e) { @@ -554,6 +555,7 @@ private void cleanupStaleIndices(Map foundIndices, Set indices, SnapshotId snapshotId, diff --git a/server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java b/server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java index 71450b576abbf..c38ddb45ab853 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java +++ b/server/src/test/java/org/elasticsearch/snapshots/mockstore/BlobContainerWrapper.java @@ -21,11 +21,11 @@ import org.elasticsearch.common.blobstore.BlobContainer; import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; +import org.elasticsearch.common.blobstore.DeleteResult; import java.io.IOException; import java.io.InputStream; import java.util.Map; -import java.util.function.LongConsumer; public class BlobContainerWrapper implements BlobContainer { private BlobContainer delegate; @@ -61,8 +61,8 @@ public void deleteBlob(String blobName) throws IOException { } @Override - public void delete(LongConsumer resultConsumer) throws IOException { - delegate.delete(resultConsumer); + public DeleteResult delete() throws IOException { + return delegate.delete(); } @Override diff --git a/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java b/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java index d61e4ae551ea2..9a2e4e246ec43 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java +++ b/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockEventuallyConsistentRepository.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.support.PlainBlobMetaData; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.util.Maps; @@ -48,8 +49,8 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; -import java.util.function.LongConsumer; import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; @@ -216,16 +217,20 @@ public void deleteBlob(String blobName) { } @Override - public void delete(LongConsumer resultConsumer) { + public DeleteResult delete() { ensureNotClosed(); final String thisPath = path.buildAsString(); + final AtomicLong bytesDeleted = new AtomicLong(0L); + final AtomicLong blobsDeleted = new AtomicLong(0L); synchronized (context.actions) { consistentView(context.actions).stream().filter(action -> action.path.startsWith(thisPath)) .forEach(a -> { context.actions.add(new BlobStoreAction(Operation.DELETE, a.path)); - resultConsumer.accept(a.data.length); + bytesDeleted.addAndGet(a.data.length); + blobsDeleted.incrementAndGet(); }); } + return new DeleteResult(blobsDeleted.get(), bytesDeleted.get()); } @Override diff --git a/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java b/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java index cc754f85de98f..db0ea856f01be 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java +++ b/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.blobstore.BlobMetaData; import org.elasticsearch.common.blobstore.BlobPath; import org.elasticsearch.common.blobstore.BlobStore; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.blobstore.fs.FsBlobContainer; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Setting; @@ -59,7 +60,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicLong; -import java.util.function.LongConsumer; import java.util.stream.Collectors; public class MockRepository extends FsRepository { @@ -331,14 +331,20 @@ public void deleteBlobIgnoringIfNotExists(String blobName) throws IOException { } @Override - public void delete(LongConsumer resultConsumer) throws IOException { + public DeleteResult delete() throws IOException { + DeleteResult deleteResult = DeleteResult.ZERO; for (BlobContainer child : children().values()) { - child.delete(resultConsumer); + deleteResult = deleteResult.add(child.delete()); } - for (String blob : listBlobs().values().stream().map(BlobMetaData::name).collect(Collectors.toList())) { + final Map blobs = listBlobs(); + long deleteBlobCount = blobs.size(); + long deleteByteCount = 0L; + for (String blob : blobs.values().stream().map(BlobMetaData::name).collect(Collectors.toList())) { deleteBlobIgnoringIfNotExists(blob); + deleteByteCount += blobs.get(blob).length(); } blobStore().blobContainer(path().parent()).deleteBlob(path().toArray()[path().toArray().length - 1]); + return deleteResult.add(new DeleteResult(deleteBlobCount, deleteByteCount)); } @Override diff --git a/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java b/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java index c98b447598c5c..a78a4b9323e4c 100644 --- a/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/repositories/AbstractThirdPartyRepositoryTestCase.java @@ -79,7 +79,7 @@ private void deleteAndAssertEmpty(BlobPath path) throws Exception { repo.threadPool().generic().execute(new ActionRunnable<>(future) { @Override protected void doRun() throws Exception { - repo.blobStore().blobContainer(path).delete(l -> {}); + repo.blobStore().blobContainer(path).delete(); future.onResponse(null); } }); diff --git a/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/AbstractCleanupTests.java b/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/AbstractCleanupTests.java index 3bdc18864debe..3d3c9a9e9ef9e 100644 --- a/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/AbstractCleanupTests.java +++ b/x-pack/snapshot-tool/src/test/java/org/elasticsearch/snapshots/AbstractCleanupTests.java @@ -63,7 +63,7 @@ private void cleanupRepository(BlobStoreRepository repository) throws Exception repository.threadPool().generic().execute(new ActionRunnable<>(future) { @Override protected void doRun() throws Exception { - repository.blobStore().blobContainer(repository.basePath()).delete(ignored -> {}); + repository.blobStore().blobContainer(repository.basePath()).delete(); future.onResponse(null); } }); From eb45c17ac5329dde0066554a89b8c9eb530caf4e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 8 Aug 2019 10:49:23 +0200 Subject: [PATCH 64/66] nicer apis --- .../repositories/gcs/GoogleCloudStorageBlobStore.java | 2 +- .../org/elasticsearch/common/blobstore/DeleteResult.java | 7 +++++++ .../repositories/RepositoryCleanupResult.java | 9 +++++---- .../repositories/blobstore/BlobStoreRepository.java | 8 ++------ .../snapshots/mockstore/MockRepository.java | 2 +- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java index af82679977c09..c42fe232b6e46 100644 --- a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java +++ b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStore.java @@ -316,7 +316,7 @@ DeleteResult deleteDirectory(String pathStr) throws IOException { bytesDeleted.addAndGet(b.getSize()); }); deleteBlobsIgnoringIfNotExists(blobsToDelete); - deleteResult = deleteResult.add(new DeleteResult(blobsDeleted.get(), bytesDeleted.get())); + deleteResult = deleteResult.add(blobsDeleted.get(), bytesDeleted.get()); page = page.getNextPage(); } while (page != null); return deleteResult; diff --git a/server/src/main/java/org/elasticsearch/common/blobstore/DeleteResult.java b/server/src/main/java/org/elasticsearch/common/blobstore/DeleteResult.java index 62ba1a588f420..9f74e31ad7d51 100644 --- a/server/src/main/java/org/elasticsearch/common/blobstore/DeleteResult.java +++ b/server/src/main/java/org/elasticsearch/common/blobstore/DeleteResult.java @@ -19,6 +19,9 @@ package org.elasticsearch.common.blobstore; +/** + * The result of deleting multiple blobs from a {@link BlobStore}. + */ public final class DeleteResult { public static final DeleteResult ZERO = new DeleteResult(0, 0); @@ -42,4 +45,8 @@ public long bytesDeleted() { public DeleteResult add(DeleteResult other) { return new DeleteResult(blobsDeleted + other.blobsDeleted(), bytesDeleted + other.bytesDeleted()); } + + public DeleteResult add(long blobs, long bytes) { + return new DeleteResult(blobsDeleted + blobs, bytesDeleted + bytes); + } } diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java index b848cb68b7a15..bec61e02ee8f2 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryCleanupResult.java @@ -20,6 +20,7 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.blobstore.DeleteResult; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -48,12 +49,12 @@ public final class RepositoryCleanupResult implements Writeable, ToXContentObjec private long blobs; private RepositoryCleanupResult() { - this(0L, 0L); + this(DeleteResult.ZERO); } - public RepositoryCleanupResult(long blobs, long bytes) { - this.blobs = blobs; - this.bytes = bytes; + public RepositoryCleanupResult(DeleteResult result) { + this.blobs = result.blobsDeleted(); + this.bytes = result.bytesDeleted(); } public RepositoryCleanupResult(StreamInput in) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index c5311ef3e0f8b..8038cd9085e0e 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -467,18 +467,14 @@ public void cleanup(long repositoryStateId, ActionListener staleRootBlobs = staleRootBlobs(repositoryData, rootBlobs.keySet()); if (survivingIndexIds.equals(foundIndices.keySet()) && staleRootBlobs.isEmpty()) { // Nothing to clean up we return - return new RepositoryCleanupResult(0L, 0L); + return new RepositoryCleanupResult(DeleteResult.ZERO); } // write new index-N blob to ensure concurrent operations will fail writeIndexGen(repositoryData, repositoryStateId); final DeleteResult deleteIndicesResult = cleanupStaleIndices(foundIndices, survivingIndexIds); List cleaned = cleanupStaleRootFiles(staleRootBlobs); - long deletedRootBytes = 0L; - for (String name : cleaned) { - deletedRootBytes += rootBlobs.get(name).length(); - } return new RepositoryCleanupResult( - deleteIndicesResult.blobsDeleted() + cleaned.size(), deletedRootBytes + deleteIndicesResult.bytesDeleted()); + deleteIndicesResult.add(cleaned.size(), cleaned.stream().mapToLong(name -> rootBlobs.get(name).length()).sum())); }); } diff --git a/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java b/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java index db0ea856f01be..bd0a5cc772fd7 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java +++ b/server/src/test/java/org/elasticsearch/snapshots/mockstore/MockRepository.java @@ -344,7 +344,7 @@ public DeleteResult delete() throws IOException { deleteByteCount += blobs.get(blob).length(); } blobStore().blobContainer(path().parent()).deleteBlob(path().toArray()[path().toArray().length - 1]); - return deleteResult.add(new DeleteResult(deleteBlobCount, deleteByteCount)); + return deleteResult.add(deleteBlobCount, deleteByteCount); } @Override From 25fb827dde6398ec6ad397aeb48be9f8977b3160 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 9 Aug 2019 14:12:52 +0200 Subject: [PATCH 65/66] CR: add note on deletes cleaning up repo as well --- docs/reference/modules/snapshots.asciidoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/reference/modules/snapshots.asciidoc b/docs/reference/modules/snapshots.asciidoc index 1727caf0d9e91..9e71fcae279dc 100644 --- a/docs/reference/modules/snapshots.asciidoc +++ b/docs/reference/modules/snapshots.asciidoc @@ -361,6 +361,10 @@ Depending on the concrete repository implementation the numbers shown for bytes be an approximation or an exact result. Any non-zero value for the number of blobs removed implies that unreferenced blobs were found and subsequently cleaned up. +Please note that most of the cleanup operations executed by this endpoint are automatically executed when deleting any snapshot from a +repository. If you regularly delete snapshots, you will in most cases not get any or only minor space savings from using this functionality +and should lower your frequency of invoking it accordingly. + [float] [[snapshots-take-snapshot]] === Snapshot From 3af846c45e22457a262365176bba998e6d5cd839 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 9 Aug 2019 19:11:42 +0200 Subject: [PATCH 66/66] add missing empty line --- .../rest/action/admin/cluster/RestCleanupRepositoryAction.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java index 45722e443f956..c7e0e566c2c03 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java @@ -36,6 +36,7 @@ * Cleans up a repository */ public class RestCleanupRepositoryAction extends BaseRestHandler { + public RestCleanupRepositoryAction(Settings settings, RestController controller) { super(settings); controller.registerHandler(POST, "/_snapshot/{repository}/_cleanup", this);