This is an automated email from the ASF dual-hosted git repository. tflobbe pushed a commit to branch branch_9x in repository https://gitbox.apache.org/repos/asf/solr.git
commit c56a297a65256f70291621f1e5333ee89d946cf1 Author: Tomas Eduardo Fernandez Lobbe <[email protected]> AuthorDate: Tue Oct 3 15:02:56 2023 -0700 SOLR-16852: Let backups have custom key/values (#1739) --- solr/CHANGES.txt | 2 + .../client/api/model/CollectionBackupDetails.java | 1 + .../solr/cloud/api/collections/BackupCmd.java | 10 +++- .../apache/solr/core/backup/BackupProperties.java | 68 ++++++++++++++++++---- .../admin/api/CreateCollectionBackupAPI.java | 7 +++ .../LocalFSCloudIncrementalBackupTest.java | 68 ++++++++++++++++++++++ .../solr/cloud/api/collections/PurgeGraphTest.java | 6 +- .../pages/collection-management.adoc | 10 +++- .../solrj/request/CollectionAdminRequest.java | 9 +++ .../collections/AbstractIncrementalBackupTest.java | 33 ++++++++++- 10 files changed, 198 insertions(+), 16 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 12f4f7c2a2a..6f2de5636dd 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -45,6 +45,8 @@ New Features * SOLR-15367: Convert "rid" functionality into a default Tracer (Alex Deparvu, David Smiley) +* SOLR-16852: Backups now allow metadata to be added as key-values (Tomás Fernández Löbbe) + Improvements --------------------- * SOLR-16490: `/admin/cores?action=backupcore` now has a v2 equivalent, available at diff --git a/solr/api/src/java/org/apache/solr/client/api/model/CollectionBackupDetails.java b/solr/api/src/java/org/apache/solr/client/api/model/CollectionBackupDetails.java index a46529baab4..94d18f8682f 100644 --- a/solr/api/src/java/org/apache/solr/client/api/model/CollectionBackupDetails.java +++ b/solr/api/src/java/org/apache/solr/client/api/model/CollectionBackupDetails.java @@ -35,4 +35,5 @@ public class CollectionBackupDetails { public String configsetName; @JsonProperty public String collectionAlias; + @JsonProperty public Map<String, String> extraProperties; } diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java index 5d4aaf7fd79..151cbeeee1a 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java @@ -29,6 +29,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Optional; import org.apache.solr.cloud.api.collections.CollectionHandlingUtils.ShardRequestTracker; import org.apache.solr.common.SolrException; @@ -68,6 +69,7 @@ public class BackupCmd implements CollApiCmds.CollectionApiCommand { this.ccc = ccc; } + @SuppressWarnings("unchecked") @Override public void call(ClusterState state, ZkNodeProps message, NamedList<Object> results) throws Exception { @@ -91,8 +93,11 @@ public class BackupCmd implements CollApiCmds.CollectionApiCommand { .getCollection(collectionName) .getConfigName(); + Map<String, String> customProps = (Map<String, String>) message.get("extraProperties"); + BackupProperties backupProperties = - BackupProperties.create(backupName, collectionName, extCollectionName, configName); + BackupProperties.create( + backupName, collectionName, extCollectionName, configName, customProps); CoreContainer cc = ccc.getCoreContainer(); try (BackupRepository repository = cc.newBackupRepository(repo)) { @@ -367,6 +372,9 @@ public class BackupCmd implements CollApiCmds.CollectionApiCommand { } aggRsp.add("indexVersion", backupProps.getIndexVersion()); aggRsp.add("startTime", backupProps.getStartTime()); + if (backupProps.getExtraProperties() != null) { + aggRsp.add("extraProperties", backupProps.getExtraProperties()); + } // Optional options for backups Optional<Integer> indexFileCount = Optional.empty(); diff --git a/solr/core/src/java/org/apache/solr/core/backup/BackupProperties.java b/solr/core/src/java/org/apache/solr/core/backup/BackupProperties.java index 1e452bb516d..dcc8509b574 100644 --- a/solr/core/src/java/org/apache/solr/core/backup/BackupProperties.java +++ b/solr/core/src/java/org/apache/solr/core/backup/BackupProperties.java @@ -48,18 +48,33 @@ import org.apache.solr.util.PropertiesInputStream; */ public class BackupProperties { + private static final String EXTRA_PROPERTY_PREFIX = "property."; + private double indexSizeMB; private int indexFileCount; - private Properties properties; + private final Properties properties; + + private final Map<String, String> extraProperties; - private BackupProperties(Properties properties) { + private BackupProperties(Properties properties, Map<String, String> extraProperties) { this.properties = properties; + if (extraProperties == null) { + extraProperties = Map.of(); + } else if (extraProperties.keySet().stream().anyMatch(String::isEmpty)) { + throw new IllegalArgumentException("Can't have an extra property with an empty key"); + } + this.extraProperties = extraProperties; } public static BackupProperties create( - String backupName, String collectionName, String extCollectionName, String configName) { - Properties properties = new Properties(); + String backupName, + String collectionName, + String extCollectionName, + String configName, + Map<String, String> extraProperties) { + final Properties properties = new Properties(); + properties.put(BackupManager.BACKUP_NAME_PROP, backupName); properties.put(BackupManager.COLLECTION_NAME_PROP, collectionName); properties.put(BackupManager.COLLECTION_ALIAS_PROP, extCollectionName); @@ -67,7 +82,7 @@ public class BackupProperties { properties.put(BackupManager.START_TIME_PROP, Instant.now().toString()); properties.put(BackupManager.INDEX_VERSION_PROP, Version.LATEST.toString()); - return new BackupProperties(properties); + return new BackupProperties(properties, extraProperties); } public static Optional<BackupProperties> readFromLatest( @@ -93,10 +108,29 @@ public class BackupProperties { repository.openInput(backupPath, fileName, IOContext.DEFAULT)), StandardCharsets.UTF_8)) { props.load(is); - return new BackupProperties(props); + Map<String, String> extraProperties = extractExtraProperties(props); + return new BackupProperties(props, extraProperties); } } + private static Map<String, String> extractExtraProperties(Properties props) { + Map<String, String> extraProperties = new HashMap<>(); + props + .entrySet() + .removeIf( + e -> { + String entryKey = e.getKey().toString(); + if (entryKey.startsWith(EXTRA_PROPERTY_PREFIX)) { + extraProperties.put( + entryKey.substring(EXTRA_PROPERTY_PREFIX.length()), + String.valueOf(e.getValue())); + return true; + } + return false; + }); + return extraProperties; + } + public List<String> getAllShardBackupMetadataFiles() { return properties.entrySet().stream() .filter(entry -> entry.getKey().toString().endsWith(".md")) @@ -128,10 +162,14 @@ public class BackupProperties { } public void store(Writer propsWriter) throws IOException { - properties.put("indexSizeMB", String.valueOf(indexSizeMB)); - properties.put("indexFileCount", String.valueOf(indexFileCount)); - properties.put(BackupManager.END_TIME_PROP, Instant.now().toString()); - properties.store(propsWriter, "Backup properties file"); + Properties propertiesCopy = (Properties) properties.clone(); + propertiesCopy.put("indexSizeMB", String.valueOf(indexSizeMB)); + propertiesCopy.put("indexFileCount", String.valueOf(indexFileCount)); + propertiesCopy.put(BackupManager.END_TIME_PROP, Instant.now().toString()); + if (extraProperties != null && !extraProperties.isEmpty()) { + extraProperties.forEach((k, v) -> propertiesCopy.put(EXTRA_PROPERTY_PREFIX + k, v)); + } + propertiesCopy.store(propsWriter, "Backup properties file"); } public String getCollection() { @@ -158,14 +196,20 @@ public class BackupProperties { return properties.getProperty(BackupManager.INDEX_VERSION_PROP); } + public Map<String, String> getExtraProperties() { + return extraProperties; + } + public Map<String, Object> getDetails() { final Map<String, Object> result = new HashMap<>(); - properties.entrySet().stream() - .forEach(entry -> result.put(entry.getKey().toString(), entry.getValue())); + properties.entrySet().forEach(entry -> result.put(entry.getKey().toString(), entry.getValue())); result.remove(BackupManager.BACKUP_NAME_PROP); result.remove(BackupManager.COLLECTION_NAME_PROP); result.put("indexSizeMB", Double.valueOf(properties.getProperty("indexSizeMB"))); result.put("indexFileCount", Integer.valueOf(properties.getProperty("indexFileCount"))); + if (extraProperties != null && !extraProperties.isEmpty()) { + result.put("extraProperties", extraProperties); + } Map<String, String> shardBackupIds = new HashMap<>(); Iterator<String> keyIt = result.keySet().iterator(); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionBackupAPI.java b/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionBackupAPI.java index 0339b29a887..d37688f2162 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionBackupAPI.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/CreateCollectionBackupAPI.java @@ -22,6 +22,7 @@ import static org.apache.solr.cloud.Overseer.QUEUE_OPERATION; import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP; import static org.apache.solr.common.params.CollectionAdminParams.FOLLOW_ALIASES; import static org.apache.solr.common.params.CollectionAdminParams.INDEX_BACKUP_STRATEGY; +import static org.apache.solr.common.params.CollectionAdminParams.PROPERTY_PREFIX; import static org.apache.solr.common.params.CommonAdminParams.ASYNC; import static org.apache.solr.common.params.CommonParams.NAME; import static org.apache.solr.common.params.CoreAdminParams.BACKUP_CONFIGSET; @@ -31,6 +32,7 @@ import static org.apache.solr.common.params.CoreAdminParams.BACKUP_REPOSITORY; import static org.apache.solr.common.params.CoreAdminParams.COMMIT_NAME; import static org.apache.solr.common.params.CoreAdminParams.MAX_NUM_BACKUP_POINTS; import static org.apache.solr.handler.admin.CollectionsHandler.DEFAULT_COLLECTION_OP_TIMEOUT; +import static org.apache.solr.handler.admin.api.CreateCollection.copyPrefixedPropertiesWithoutPrefix; import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM; import com.fasterxml.jackson.annotation.JsonProperty; @@ -166,6 +168,9 @@ public class CreateCollectionBackupAPI extends BackupAPIBase { requestBody.incremental = params.getBool(BACKUP_INCREMENTAL); requestBody.backupConfigset = params.getBool(BACKUP_CONFIGSET); requestBody.maxNumBackupPoints = params.getInt(MAX_NUM_BACKUP_POINTS); + requestBody.extraProperties = + copyPrefixedPropertiesWithoutPrefix(params, new HashMap<>(), PROPERTY_PREFIX); + requestBody.async = params.get(ASYNC); return requestBody; @@ -192,6 +197,7 @@ public class CreateCollectionBackupAPI extends BackupAPIBase { @JsonProperty public Boolean backupConfigset; @JsonProperty public Integer maxNumBackupPoints; @JsonProperty public String async; + @JsonProperty public Map<String, String> extraProperties; } public static class CreateCollectionBackupResponseBody @@ -215,6 +221,7 @@ public class CreateCollectionBackupAPI extends BackupAPIBase { @JsonProperty public Integer indexFileCount; @JsonProperty public Integer uploadedIndexFileCount; @JsonProperty public Double indexSizeMB; + @JsonProperty public Map<String, String> extraProperties; @JsonProperty("uploadedIndexFileMB") public Double uploadedIndexSizeMB; diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/LocalFSCloudIncrementalBackupTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/LocalFSCloudIncrementalBackupTest.java index 1c1f3536b76..06b9ff3e0df 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/LocalFSCloudIncrementalBackupTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/LocalFSCloudIncrementalBackupTest.java @@ -17,8 +17,19 @@ package org.apache.solr.cloud.api.collections; +import java.util.List; +import java.util.Map; +import java.util.Properties; import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.response.CollectionAdminResponse; +import org.apache.solr.cloud.AbstractDistribZkTestBase; +import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.core.backup.repository.BackupRepository; import org.junit.BeforeClass; +import org.junit.Test; // Backups do checksum validation against a footer value not present in 'SimpleText' @LuceneTestCase.SuppressCodecs({"SimpleText"}) @@ -85,4 +96,61 @@ public class LocalFSCloudIncrementalBackupTest extends AbstractIncrementalBackup public String getBackupLocation() { return backupLocation; } + + @SuppressWarnings("unchecked") + @Test + public void testCustomProperties() throws Exception { + setTestSuffix("testCustomProperties"); + final String backupCollectionName = getCollectionName(); + final String restoreCollectionName = backupCollectionName + "_restore"; + + CloudSolrClient solrClient = cluster.getSolrClient(); + + CollectionAdminRequest.createCollection(backupCollectionName, "conf1", NUM_SHARDS, 1) + .process(solrClient); + int numDocs = indexDocs(backupCollectionName, true); + String backupName = BACKUPNAME_PREFIX + testSuffix; + try (BackupRepository repository = + cluster.getJettySolrRunner(0).getCoreContainer().newBackupRepository(BACKUP_REPO_NAME)) { + String backupLocation = repository.getBackupLocation(getBackupLocation()); + Properties extraProps = new Properties(); + extraProps.putAll(Map.of("foo", "bar", "number", "12345")); + CollectionAdminRequest.backupCollection(backupCollectionName, backupName) + .setLocation(backupLocation) + .setExtraProperties(extraProps) + .setRepositoryName(BACKUP_REPO_NAME) + .processAndWait(cluster.getSolrClient(), 100); + CollectionAdminResponse response = + CollectionAdminRequest.listBackup(backupName) + .setBackupLocation(backupLocation) + .setBackupRepository(BACKUP_REPO_NAME) + .process(cluster.getSolrClient()); + assertNotNull(response.getResponse().get("backups")); + assertTrue(response.getResponse().get("backups") instanceof List); + List<Map<String, Object>> backups = + (List<Map<String, Object>>) response.getResponse().get("backups"); + assertEquals(1, backups.size()); + Map<String, Object> backup0 = backups.get(0); + assertNotNull(backup0.get("extraProperties")); + assertTrue(backup0.get("extraProperties") instanceof Map); + Map<String, Object> extraProperties = (Map<String, Object>) backup0.get("extraProperties"); + assertEquals("bar", extraProperties.get("foo")); + assertEquals("12345", extraProperties.get("number")); + + CollectionAdminRequest.restoreCollection(restoreCollectionName, backupName) + .setLocation(backupLocation) + .setRepositoryName(BACKUP_REPO_NAME) + .processAndWait(solrClient, 500); + + AbstractDistribZkTestBase.waitForRecoveriesToFinish( + restoreCollectionName, ZkStateReader.from(solrClient), false, false, 3); + assertEquals( + numDocs, + cluster + .getSolrClient() + .query(restoreCollectionName, new SolrQuery("*:*")) + .getResults() + .getNumFound()); + } + } } diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/PurgeGraphTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/PurgeGraphTest.java index 68599125f38..b2374f89a57 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/PurgeGraphTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/PurgeGraphTest.java @@ -199,7 +199,11 @@ public class PurgeGraphTest extends SolrTestCaseJ4 { private void createBackupIdFile(int backupId, String... shardNames) throws Exception { final BackupProperties createdProps = BackupProperties.create( - "someBackupName", "someCollectionName", "someExtCollectionName", "someConfigName"); + "someBackupName", + "someCollectionName", + "someExtCollectionName", + "someConfigName", + null); for (String shardName : shardNames) { createdProps.putAndGetShardBackupIdFor(shardName, backupId); } diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc index 12137859df3..107b4849209 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc @@ -1668,11 +1668,19 @@ This parameter has no effect if `incremental=false` is specified. + [%autowidth,frame=none] |=== -|Optional |Default: true +|Optional |Default: `true` |=== + Indicates if configset files should be included with the index backup or not. Note that in order to restore a collection, the configset must either exist in ZooKeeper or be part of the backup. Only set this to `false` if you can restore configsets by other means external to Solr (i.e. you have it stored with your application source code, is part of your ZooKeeper backups, etc). +`property.<propertyName>` (V1), `extraProperties` (V2):: +[%autowidth,frame=none] +|=== +|Optional |Default: none +|=== ++ +Allows storing additional key/value pairs for custom information related to the backup. In v2, the value is a map of key-value pairs. + `incremental`:: + [%autowidth,frame=none] diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java index c7814aab55b..db3ad4543c6 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java @@ -1111,6 +1111,7 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse> protected boolean incremental = true; protected Optional<Integer> maxNumBackupPoints = Optional.empty(); protected boolean backupConfigset = true; + protected Properties extraProperties; public Backup(String collection, String name) { super(CollectionAction.BACKUP, collection); @@ -1205,9 +1206,17 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse> return this; } + public Backup setExtraProperties(Properties extraProperties) { + this.extraProperties = extraProperties; + return this; + } + @Override public SolrParams getParams() { ModifiableSolrParams params = (ModifiableSolrParams) super.getParams(); + if (extraProperties != null) { + addProperties(params, extraProperties); + } params.set(CoreAdminParams.COLLECTION, collection); params.set(CoreAdminParams.NAME, name); params.set(CoreAdminParams.BACKUP_LOCATION, location); // note: optional diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java index 3e8a8d9dbae..03acadab7ff 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java @@ -21,8 +21,11 @@ import static org.apache.solr.core.TrackingBackupRepository.copiedFiles; import java.io.IOException; import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.Writer; import java.lang.invoke.MethodHandles; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; @@ -456,6 +459,34 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase { } } + public void testBackupProperties() throws IOException { + BackupProperties p = + BackupProperties.create( + "backupName", + "collection1", + "collection1-ext", + "conf1", + Map.of("foo", "bar", "aaa", "bbb")); + try (BackupRepository repository = + cluster.getJettySolrRunner(0).getCoreContainer().newBackupRepository(BACKUP_REPO_NAME)) { + String backupLocation = repository.getBackupLocation(getBackupLocation()); + URI dest = repository.resolve(repository.createURI(backupLocation), "props-file.properties"); + try (Writer propsWriter = + new OutputStreamWriter(repository.createOutput(dest), StandardCharsets.UTF_8)) { + p.store(propsWriter); + } + BackupProperties propsRead = + BackupProperties.readFrom( + repository, repository.createURI(backupLocation), "props-file.properties"); + assertEquals(p.getCollection(), propsRead.getCollection()); + assertEquals(p.getCollectionAlias(), propsRead.getCollectionAlias()); + assertEquals(p.getConfigName(), propsRead.getConfigName()); + assertEquals(p.getIndexVersion(), propsRead.getIndexVersion()); + assertEquals(p.getExtraProperties(), propsRead.getExtraProperties()); + assertEquals(p.getBackupName(), propsRead.getBackupName()); + } + } + protected void corruptIndexFiles() throws IOException { List<Slice> slices = new ArrayList<>(getCollectionState(getCollectionName()).getSlices()); Replica leader = slices.get(random().nextInt(slices.size())).getLeader(); @@ -550,7 +581,7 @@ public abstract class AbstractIncrementalBackupTest extends SolrCloudTestCase { log.info("Indexed {} docs to collection: {}", numDocs, collectionName); } - private int indexDocs(String collectionName, boolean useUUID) throws Exception { + protected int indexDocs(String collectionName, boolean useUUID) throws Exception { Random random = new Random( docsSeed); // use a constant seed for the whole test run so that we can easily re-index.
