dsmiley commented on code in PR #2039:
URL: https://github.com/apache/solr/pull/2039#discussion_r1410728489
##########
solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCollectionAPI.java:
##########
@@ -70,15 +69,17 @@
@Path("/backups/{backupName}/restore")
public class RestoreCollectionAPI extends BackupAPIBase {
- private static final Set<String> CREATE_PARAM_ALLOWLIST =
- Set.of(
- COLL_CONF,
- REPLICATION_FACTOR,
- NRT_REPLICAS,
- TLOG_REPLICAS,
- PULL_REPLICAS,
- CREATE_NODE_SET_PARAM,
- CREATE_NODE_SET_SHUFFLE_PARAM);
+ private static final Set<String> CREATE_PARAM_ALLOWLIST =
makeCreateParamAllowList();
+
+ private static Set<String> makeCreateParamAllowList() {
Review Comment:
Just playing around here... consider this code:
```
Stream.concat(
CollectionHandlingUtils.numReplicasProperties().stream(),
Stream.of(COLL_CONF, REPLICATION_FACTOR, CREATE_NODE_SET_PARAM,
CREATE_NODE_SET_SHUFFLE_PARAM))
.collect(Collectors.toUnmodifiableSet());
```
##########
solr/solrj/src/java/org/apache/solr/common/cloud/ReplicaCount.java:
##########
@@ -100,6 +166,10 @@ public int total() {
return countByType.values().stream().reduce(Integer::sum).orElse(0);
}
+ public int count(EnumSet<Replica.Type> replicaTypes) {
Review Comment:
Agree with @murblanc . This is very light logic that deals directly with
the types at hand.
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java:
##########
@@ -35,19 +31,21 @@
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.ReplicaCount;
import org.apache.solr.common.cloud.ZkNodeProps;
-import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CommonAdminParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.Utils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class CreateShardCmd implements CollApiCmds.CollectionApiCommand {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private final CollectionCommandContext ccc;
+ private final EnumSet<Replica.Type> leaderEligibleReplicaTypes;
Review Comment:
could be static or removed? Seems like just a convenient reference.
##########
solr/solrj/src/java/org/apache/solr/common/cloud/ReplicaCount.java:
##########
@@ -61,6 +152,33 @@ public boolean contains(Replica.Type type) {
return countByType.containsKey(type);
}
+ /** Returns the replica types for which a number of replicas was explicitely
defined. */
+ public Set<Replica.Type> keySet() {
+ return countByType.keySet();
+ }
+
+ /**
+ * Add properties for replica counts as integers to a properties map.
+ *
+ * @param propMap a properties map.
+ */
+ public void writeProps(Map<String, Object> propMap) {
+ for (Map.Entry<Replica.Type, Integer> entry : countByType.entrySet()) {
+ propMap.put(entry.getKey().numReplicasPropertyName, entry.getValue());
+ }
+ }
+
+ /**
+ * Add properties for replica counts as integers to Solr parameters.
+ *
+ * @param params a set of modifiable Solr parameters.
+ */
+ public void writeProps(ModifiableSolrParams params) {
+ for (Map.Entry<Replica.Type, Integer> entry : countByType.entrySet()) {
+ params.add(entry.getKey().numReplicasPropertyName,
String.valueOf(entry.getValue()));
Review Comment:
More correct to use params.set, and you needn't add the valueOf part because
`set` is overloaded with an int variant.
##########
solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java:
##########
@@ -103,31 +104,41 @@ public enum Type {
* support NRT (soft commits) and RTG. Any {@link Type#NRT} replica can
become a leader. A shard
* leader will forward updates to all active {@link Type#NRT} and {@link
Type#TLOG} replicas.
*/
- NRT(true),
+ NRT(true, CollectionAdminParams.NRT_REPLICAS),
/**
* Writes to transaction log, but not to index, uses replication. Any
{@link Type#TLOG} replica
* can become leader (by first applying all local transaction log
elements). If a replica is of
* type {@link Type#TLOG} but is also the leader, it will behave as a
{@link Type#NRT}. A shard
* leader will forward updates to all active {@link Type#NRT} and {@link
Type#TLOG} replicas.
*/
- TLOG(true),
+ TLOG(true, CollectionAdminParams.TLOG_REPLICAS),
/**
* Doesn’t index or writes to transaction log. Just replicates from {@link
Type#NRT} or {@link
* Type#TLOG} replicas. {@link Type#PULL} replicas can’t become shard
leaders (i.e., if there
* are only pull replicas in the collection at some point, updates will
fail same as if there is
* no leaders, queries continue to work), so they don’t even participate
in elections.
*/
- PULL(false);
+ PULL(false, CollectionAdminParams.PULL_REPLICAS);
public final boolean leaderEligible;
+ public final String numReplicasPropertyName;
- Type(boolean b) {
- this.leaderEligible = b;
+ Type(boolean leaderEligible, String numReplicasPropertyName) {
+ this.leaderEligible = leaderEligible;
+ this.numReplicasPropertyName = numReplicasPropertyName;
}
public static Type get(String name) {
return StrUtils.isNullOrEmpty(name) ? NRT :
Type.valueOf(name.toUpperCase(Locale.ROOT));
}
+
+ /**
+ * Returns a default replica type. It is most notably used by the replica
factor, which maps
+ * onto this replica type. This replica type needs to be leader-eligible.
+ */
+ public static Type defaultType() {
Review Comment:
If we introduce this, then wouldn't the default replica counts on creating a
collection potentially switch from NRT to an algorithmic/dynamic detection
based on this?
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java:
##########
@@ -135,6 +140,20 @@ public class CollectionHandlingUtils {
}
}
+ /** Returns names of properties that are used to specify a number of
replicas of a given type. */
Review Comment:
I agree with @murblanc . It's really tiny amounts of code any way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
