From b06ddb617c09f65a7804c483d3e3dcc7159c4a29 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 4 Oct 2024 12:48:14 -0400 Subject: [PATCH 1/2] Fix warnings from SLF4J on startup when repository-s3 is installed (#16194) * Fix warnings from SLF4J on startup when repository-s3 is installed Signed-off-by: Craig Perkins * Add to CHANGELOG Signed-off-by: Craig Perkins * Fix precommit Signed-off-by: Craig Perkins --------- Signed-off-by: Craig Perkins --- CHANGELOG.md | 1 + plugins/repository-s3/build.gradle | 7 ++++--- .../licenses/log4j-slf4j-impl-2.21.0.jar.sha1 | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) create mode 100644 plugins/repository-s3/licenses/log4j-slf4j-impl-2.21.0.jar.sha1 diff --git a/CHANGELOG.md b/CHANGELOG.md index ca23cd59e06d5..1a289a8c62000 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix infinite loop in nested agg ([#15931](https://github.com/opensearch-project/OpenSearch/pull/15931)) - Fix race condition in node-join and node-left ([#15521](https://github.com/opensearch-project/OpenSearch/pull/15521)) - Streaming bulk request hangs ([#16158](https://github.com/opensearch-project/OpenSearch/pull/16158)) +- Fix warnings from SLF4J on startup when repository-s3 is installed ([#16194](https://github.com/opensearch-project/OpenSearch/pull/16194)) ### Security diff --git a/plugins/repository-s3/build.gradle b/plugins/repository-s3/build.gradle index 00decbe4fa9cd..22aa151c92003 100644 --- a/plugins/repository-s3/build.gradle +++ b/plugins/repository-s3/build.gradle @@ -82,6 +82,8 @@ dependencies { api "joda-time:joda-time:${versions.joda}" api "org.slf4j:slf4j-api:${versions.slf4j}" + runtimeOnly "org.apache.logging.log4j:log4j-slf4j-impl:${versions.log4j}" + // network stack api "io.netty:netty-buffer:${versions.netty}" api "io.netty:netty-codec:${versions.netty}" @@ -111,6 +113,7 @@ tasks.named("dependencyLicenses").configure { mapping from: /jackson-.*/, to: 'jackson' mapping from: /jaxb-.*/, to: 'jaxb' mapping from: /netty-.*/, to: 'netty' + mapping from: /log4j-.*/, to: 'log4j' } bundlePlugin { @@ -510,9 +513,7 @@ thirdPartyAudit { 'org.jboss.marshalling.MarshallingConfiguration', 'org.jboss.marshalling.Unmarshaller', - 'org.slf4j.impl.StaticLoggerBinder', - 'org.slf4j.impl.StaticMDCBinder', - 'org.slf4j.impl.StaticMarkerBinder', + 'org.slf4j.ext.EventData', 'reactor.blockhound.BlockHound$Builder', 'reactor.blockhound.integration.BlockHoundIntegration', diff --git a/plugins/repository-s3/licenses/log4j-slf4j-impl-2.21.0.jar.sha1 b/plugins/repository-s3/licenses/log4j-slf4j-impl-2.21.0.jar.sha1 new file mode 100644 index 0000000000000..0e22f98daa61c --- /dev/null +++ b/plugins/repository-s3/licenses/log4j-slf4j-impl-2.21.0.jar.sha1 @@ -0,0 +1 @@ +911fdb5b1a1df36719c579ecc6f2957b88bce1ab \ No newline at end of file From 421a1cc2270ef02f02fc7621895f8b15ec7282f1 Mon Sep 17 00:00:00 2001 From: Sachin Kale Date: Fri, 4 Oct 2024 22:24:36 +0530 Subject: [PATCH 2/2] Fix unknown parameter source_remote_translog_repository bug (#16192) Signed-off-by: Sachin Kale --- .../RestoreShallowSnapshotV2IT.java | 83 +++++++++++++++++++ .../restore/RestoreSnapshotRequest.java | 6 ++ .../RestoreSnapshotRequestBuilder.java | 8 ++ 3 files changed, 97 insertions(+) diff --git a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java index c5a55f16cab2b..24f1141ddbede 100644 --- a/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java +++ b/server/src/internalClusterTest/java/org/opensearch/remotestore/RestoreShallowSnapshotV2IT.java @@ -10,6 +10,8 @@ import org.opensearch.action.DocWriteResponse; import org.opensearch.action.admin.cluster.remotestore.restore.RestoreRemoteStoreRequest; +import org.opensearch.action.admin.cluster.repositories.get.GetRepositoriesRequest; +import org.opensearch.action.admin.cluster.repositories.get.GetRepositoriesResponse; import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; import org.opensearch.action.admin.indices.recovery.RecoveryResponse; @@ -19,6 +21,7 @@ import org.opensearch.client.Requests; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.RepositoryMetadata; import org.opensearch.cluster.routing.RecoverySource; import org.opensearch.common.Nullable; import org.opensearch.common.blobstore.BlobPath; @@ -64,6 +67,7 @@ import static org.opensearch.index.remote.RemoteStoreEnums.DataType.DATA; import static org.opensearch.index.remote.RemoteStoreEnums.DataType.METADATA; import static org.opensearch.indices.RemoteStoreSettings.CLUSTER_REMOTE_STORE_PATH_TYPE_SETTING; +import static org.opensearch.repositories.blobstore.BlobStoreRepository.SYSTEM_REPOSITORY_SETTING; import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; @@ -106,6 +110,18 @@ protected Settings.Builder getRepositorySettings(Path location, boolean shallowC return settingsBuilder; } + protected Settings.Builder getRepositorySettings(String sourceRepository, boolean readOnly) throws ExecutionException, + InterruptedException { + GetRepositoriesRequest gr = new GetRepositoriesRequest(new String[] { sourceRepository }); + GetRepositoriesResponse res = client().admin().cluster().getRepositories(gr).get(); + RepositoryMetadata rmd = res.repositories().get(0); + return Settings.builder() + .put(rmd.settings()) + .put(BlobStoreRepository.READONLY_SETTING.getKey(), readOnly) + .put(BlobStoreRepository.SHALLOW_SNAPSHOT_V2.getKey(), false) + .put(SYSTEM_REPOSITORY_SETTING.getKey(), false); + } + private Settings.Builder getIndexSettings(int numOfShards, int numOfReplicas) { Settings.Builder settingsBuilder = Settings.builder() .put(super.indexSettings()) @@ -802,4 +818,71 @@ public void testInvalidRestoreRequestScenarios() throws Exception { ); assertTrue(exception.getMessage().contains("cannot remove setting [index.remote_store.segment.repository]" + " on restore")); } + + public void testRestoreOperationsUsingDifferentRepos() throws Exception { + disableRepoConsistencyCheck("Remote store repo"); + String clusterManagerNode = internalCluster().startClusterManagerOnlyNode(); + String primary = internalCluster().startDataOnlyNode(); + String indexName1 = "testindex1"; + String snapshotRepoName = "test-snapshot-repo"; + String snapshotName1 = "test-snapshot1"; + Path absolutePath1 = randomRepoPath().toAbsolutePath(); + logger.info("Snapshot Path [{}]", absolutePath1); + + // Create repo + createRepository(snapshotRepoName, "fs", getRepositorySettings(absolutePath1, true)); + + // Create index + Client client = client(); + Settings indexSettings = getIndexSettings(1, 0).build(); + createIndex(indexName1, indexSettings); + ensureGreen(indexName1); + + // Index 5 documents, refresh, index 5 documents + final int numDocsInIndex1 = 5; + indexDocuments(client, indexName1, 0, numDocsInIndex1); + refresh(indexName1); + indexDocuments(client, indexName1, numDocsInIndex1, 2 * numDocsInIndex1); + + // Take V2 snapshot + logger.info("--> snapshot"); + SnapshotInfo snapshotInfo = createSnapshot(snapshotRepoName, snapshotName1, new ArrayList<>()); + assertThat(snapshotInfo.state(), equalTo(SnapshotState.SUCCESS)); + assertThat(snapshotInfo.successfulShards(), greaterThan(0)); + assertThat(snapshotInfo.successfulShards(), equalTo(snapshotInfo.totalShards())); + + // Create new snapshot, segment and translog repositories + String newSnapshotRepo = "backup-snapshot"; + String newSegmentRepo = "backup-segment"; + String newTranslogRepo = "backup-translog"; + createRepository(newSnapshotRepo, "fs", getRepositorySettings(snapshotRepoName, true)); + createRepository(newSegmentRepo, "fs", getRepositorySettings(BASE_REMOTE_REPO, true)); + createRepository(newTranslogRepo, "fs", getRepositorySettings(BASE_REMOTE_REPO, true)); + + // Delete index + assertAcked(client().admin().indices().delete(new DeleteIndexRequest(indexName1)).get()); + assertFalse(indexExists(indexName1)); + + // Restore using new repos + RestoreSnapshotResponse restoreSnapshotResponse1 = client.admin() + .cluster() + .prepareRestoreSnapshot(newSnapshotRepo, snapshotName1) + .setWaitForCompletion(false) + .setIndices(indexName1) + .setSourceRemoteStoreRepository(newSegmentRepo) + .setSourceRemoteTranslogRepository(newTranslogRepo) + .get(); + + assertEquals(restoreSnapshotResponse1.status(), RestStatus.ACCEPTED); + + // Verify restored index's stats + ensureYellowAndNoInitializingShards(indexName1); + ensureGreen(indexName1); + assertDocsPresentInIndex(client(), indexName1, 2 * numDocsInIndex1); + + // indexing some new docs and validating + indexDocuments(client, indexName1, 2 * numDocsInIndex1, 3 * numDocsInIndex1); + ensureGreen(indexName1); + assertDocsPresentInIndex(client, indexName1, 3 * numDocsInIndex1); + } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java index 469a31407dc5d..f3110cc8f20a5 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java @@ -652,6 +652,12 @@ public RestoreSnapshotRequest source(Map source) { } else { throw new IllegalArgumentException("malformed source_remote_store_repository"); } + } else if (name.equals("source_remote_translog_repository")) { + if (entry.getValue() instanceof String) { + setSourceRemoteTranslogRepository((String) entry.getValue()); + } else { + throw new IllegalArgumentException("malformed source_remote_translog_repository"); + } } else { if (IndicesOptions.isIndicesOptions(name) == false) { throw new IllegalArgumentException("Unknown parameter " + name); diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestBuilder.java b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestBuilder.java index 39eaadf3c8de6..53c9557a621b7 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestBuilder.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestBuilder.java @@ -266,4 +266,12 @@ public RestoreSnapshotRequestBuilder setSourceRemoteStoreRepository(String repos request.setSourceRemoteStoreRepository(repositoryName); return this; } + + /** + * Sets the source remote translog repository name + */ + public RestoreSnapshotRequestBuilder setSourceRemoteTranslogRepository(String repositoryName) { + request.setSourceRemoteTranslogRepository(repositoryName); + return this; + } }