SOLR-16852: Let backups have custom key/values #1739
Conversation
|
|
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| "conf1", | ||
| Map.of("foo", "bar", "aaa", "bbb")); | ||
| try (BackupRepository repository = | ||
| cluster.getJettySolrRunner(0).getCoreContainer().newBackupRepository(BACKUP_REPO_NAME)) { |
There was a problem hiding this comment.
NULL_DEREFERENCE: object returned by org.apache.solr.cloud.api.collections.AbstractIncrementalBackupTest.cluster.getJettySolrRunner(0).getCoreContainer() could be null and is dereferenced at line 472.
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
| "conf1", | ||
| Map.of("foo", "bar", "aaa", "bbb")); | ||
| try (BackupRepository repository = | ||
| cluster.getJettySolrRunner(0).getCoreContainer().newBackupRepository(BACKUP_REPO_NAME)) { |
There was a problem hiding this comment.
NULLPTR_DEREFERENCE: CoreContainer JettySolrRunner.getCoreContainer() could be null (from the call to JettySolrRunner.getCoreContainer() on line 472) and is dereferenced.
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
| @@ -550,7 +582,7 @@ private void indexDocs(String collectionName, int numDocs, boolean useUUID) thro | |||
| 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 = | |||
There was a problem hiding this comment.
PREDICTABLE_RANDOM: This random generator (java.util.Random) is predictable
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
| @@ -550,7 +582,7 @@ private void indexDocs(String collectionName, int numDocs, boolean useUUID) thro | |||
There was a problem hiding this comment.
NULL_DEREFERENCE: object leader last assigned on line 493 could be null and is dereferenced by call to getReplicaJetty(...) at line 494.
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
| @@ -550,7 +582,7 @@ private void indexDocs(String collectionName, int numDocs, boolean useUUID) thro | |||
There was a problem hiding this comment.
NULLPTR_DEREFERENCE: leader could be null (from the call to Slice.getLeader() on line 493) and is dereferenced in the call to MiniSolrCloudCluster.getReplicaJetty(...).
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
tflobbe
force-pushed
the
backup-props
branch
from
ee88689
to
1ff2936
Compare
|
@dsmiley, Any concerns with this approach, given the discussion in the Jira issue? |
tflobbe
force-pushed
the
backup-props
branch
from
1ff2936
to
b4ab31b
Compare
| if (entryKey.startsWith(EXTRA_PROPERTY_PREFIX)) { | ||
| extraProperties.put( | ||
| entryKey.substring(EXTRA_PROPERTY_PREFIX.length()), String.valueOf(value)); | ||
| props.remove(key); |
There was a problem hiding this comment.
I don't think you can iterate & remove in the same loop -- ConcurrentModificationException ?
There was a problem hiding this comment.
Properties are backed by a ConcurrentHashMap, so it should be OK
There was a problem hiding this comment.
Yeah, okay, although depending on internal details :-/. Another alternative is using props.entrySet().removeIf
Any way, doesn't matter much to me.
There was a problem hiding this comment.
Yes, I agree that's cleaner. I'll update
| 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(); |
There was a problem hiding this comment.
nice; it was sketchy previously, modifying the field properties here
Currently backups have some properties attached such as
configName,collectionAlias,indexVersion, etc. It would be useful to allow custom properties/annotations to add some information coming from the process that triggers the backup. For example, say you have a process that consumes from a queue, adds documents to Solr and then triggers a backup command, the backup could include the queue offset at the time of triggering the backup so that the application could potentially continue processing from such offset upon restore.