Skip to content

SOLR-17779: Remove 'roles' and 'numShards' core properties #3420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ Deprecation Removals

* SOLR-17778: Removed no-op core admin operation FORCEPREPAREFORLEADERSHIP and its SolrJ peer OverrideLastPublished. (Pierre Salagnac)

* SOLR-17779: Removed obsolete 'roles' and 'numShards' core properties, which were persisted with but unused (Pierre Salagnac).

Dependency Upgrades
---------------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import com.fasterxml.jackson.annotation.JsonProperty;
import io.swagger.v3.oas.annotations.media.Schema;
import java.util.List;
import java.util.Map;

public class CreateCoreParams {
Expand Down Expand Up @@ -47,8 +46,6 @@ public class CreateCoreParams {

@JsonProperty public String collection;

@JsonProperty public List<String> roles;

@JsonProperty public String replicaType;

@JsonProperty public Map<String, String> properties;
Expand Down
28 changes: 0 additions & 28 deletions solr/core/src/java/org/apache/solr/cloud/CloudDescriptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.Map;
import java.util.Properties;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.util.PropertiesUtil;
import org.apache.solr.common.util.StrUtils;
import org.apache.solr.core.CoreDescriptor;

Expand All @@ -31,8 +30,6 @@ public class CloudDescriptor {

private String shardId;
private String collectionName;
private String roles = null;
private Integer numShards;
private String nodeName = null;
private Map<String, String> collectionParams = new HashMap<>();

Expand All @@ -43,8 +40,6 @@ public class CloudDescriptor {
private volatile boolean hasRegistered = false;
private volatile Replica.State lastPublished = Replica.State.ACTIVE;

public static final String NUM_SHARDS = "numShards";

public static final String REPLICA_TYPE = "replicaType";

/** The type of replica this core hosts */
Expand All @@ -56,10 +51,8 @@ public CloudDescriptor(CoreDescriptor cd, String coreName, Properties props) {
if (StrUtils.isNullOrEmpty(shardId)) this.shardId = null;
// If no collection name is specified, we default to the core name
this.collectionName = props.getProperty(CoreDescriptor.CORE_COLLECTION, coreName);
this.roles = props.getProperty(CoreDescriptor.CORE_ROLES, null);
this.nodeName = props.getProperty(CoreDescriptor.CORE_NODE_NAME);
if (StrUtils.isNullOrEmpty(nodeName)) this.nodeName = null;
this.numShards = PropertiesUtil.toInteger(props.getProperty(CloudDescriptor.NUM_SHARDS), null);
this.replicaType = Replica.Type.get(props.getProperty(CloudDescriptor.REPLICA_TYPE));
for (String propName : props.stringPropertyNames()) {
if (propName.startsWith(ZkController.COLLECTION_PARAM_PREFIX)) {
Expand Down Expand Up @@ -110,28 +103,11 @@ public void setCollectionName(String collectionName) {
this.collectionName = collectionName;
}

public String getRoles() {
return roles;
}

public void setRoles(String roles) {
this.roles = roles;
}

/** Optional parameters that can change how a core is created. */
public Map<String, String> getParams() {
return collectionParams;
}

// setting only matters on core creation
public Integer getNumShards() {
return numShards;
}

public void setNumShards(int numShards) {
this.numShards = numShards;
}

public String getCoreNodeName() {
return nodeName;
}
Expand All @@ -152,10 +128,6 @@ public void reload(CloudDescriptor reloadFrom) {
StrUtils.isNullOrEmpty(reloadFrom.getCollectionName())
? getCollectionName()
: reloadFrom.getCollectionName());
setRoles(StrUtils.isNullOrEmpty(reloadFrom.getRoles()) ? getRoles() : reloadFrom.getRoles());
if (reloadFrom.getNumShards() != null) {
setNumShards(reloadFrom.getNumShards());
}
setCoreNodeName(
StrUtils.isNullOrEmpty(reloadFrom.getCoreNodeName())
? getCoreNodeName()
Expand Down
5 changes: 0 additions & 5 deletions solr/core/src/java/org/apache/solr/cloud/ZkController.java
Original file line number Diff line number Diff line change
Expand Up @@ -1755,7 +1755,6 @@ public void publish(

log.debug("publishing state={}", state);
// System.out.println(Thread.currentThread().getStackTrace()[3]);
Integer numShards = cd.getCloudDescriptor().getNumShards();

assert collection != null && collection.length() > 0;

Expand All @@ -1768,7 +1767,6 @@ public void publish(
props.put(Overseer.QUEUE_OPERATION, OverseerAction.STATE.toLower());
props.put(ZkStateReader.STATE_PROP, state.toString());
props.put(ZkStateReader.CORE_NAME_PROP, cd.getName());
props.put(ZkStateReader.ROLES_PROP, cd.getCloudDescriptor().getRoles());
props.put(ZkStateReader.NODE_NAME_PROP, getNodeName());
props.put(
ZkStateReader.BASE_URL_PROP, zkStateReader.getBaseUrlForNodeName(getNodeName()));
Expand All @@ -1777,9 +1775,6 @@ public void publish(
props.put(
ZkStateReader.REPLICA_TYPE, cd.getCloudDescriptor().getReplicaType().toString());
props.put(ZkStateReader.FORCE_SET_STATE_PROP, "false");
if (numShards != null) {
props.put(ZkStateReader.NUM_SHARDS_PROP, numShards.toString());
}
props.putIfNotNull(ZkStateReader.CORE_NODE_NAME_PROP, coreNodeName);
};

Expand Down
5 changes: 1 addition & 4 deletions solr/core/src/java/org/apache/solr/core/CoreDescriptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ public class CoreDescriptor {
public static final String CORE_SCHEMA = "schema";
public static final String CORE_SHARD = "shard";
public static final String CORE_COLLECTION = "collection";
public static final String CORE_ROLES = "roles";
public static final String CORE_PROPERTIES = "properties";
public static final String CORE_LOADONSTARTUP = "loadOnStartup";
public static final String CORE_TRANSIENT = "transient";
Expand Down Expand Up @@ -110,9 +109,7 @@ public Properties getPersistableUserProperties() {
// cloud props
CORE_SHARD,
CORE_COLLECTION,
CORE_ROLES,
CORE_NODE_NAME,
CloudDescriptor.NUM_SHARDS);
CORE_NODE_NAME);

private final CloudDescriptor cloudDesc;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public class ClusterStatus {
public static final String INCLUDE_ALL = "includeAll";
public static final String LIVENODES_PROP = "liveNodes";
public static final String CLUSTER_PROP = "clusterProperties";
public static final String ROLES_PROP = "roles";
public static final String ALIASES_PROP = "aliases";

/** Shard / collection health state. */
Expand Down Expand Up @@ -108,7 +109,7 @@ public void getClusterStatus(NamedList<Object> results, SolrVersion solrVersion)
boolean includeAll = solrParams.getBool(INCLUDE_ALL, true);
boolean withLiveNodes = solrParams.getBool(LIVENODES_PROP, includeAll);
boolean withClusterProperties = solrParams.getBool(CLUSTER_PROP, includeAll);
boolean withRoles = solrParams.getBool(ZkStateReader.ROLES_PROP, includeAll);
boolean withRoles = solrParams.getBool(ROLES_PROP, includeAll);
boolean withCollection = includeAll || (collection != null);
boolean withAliases = solrParams.getBool(ALIASES_PROP, includeAll);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import org.apache.solr.cloud.CloudDescriptor;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CommonAdminParams;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.params.ModifiableSolrParams;
Expand Down Expand Up @@ -271,9 +270,7 @@ protected void handleCustomAction(SolrQueryRequest req, SolrQueryResponse rsp) {
Map.entry(CoreAdminParams.TRANSIENT, CoreDescriptor.CORE_TRANSIENT),
Map.entry(CoreAdminParams.SHARD, CoreDescriptor.CORE_SHARD),
Map.entry(CoreAdminParams.COLLECTION, CoreDescriptor.CORE_COLLECTION),
Map.entry(CoreAdminParams.ROLES, CoreDescriptor.CORE_ROLES),
Map.entry(CoreAdminParams.CORE_NODE_NAME, CoreDescriptor.CORE_NODE_NAME),
Map.entry(ZkStateReader.NUM_SHARDS_PROP, CloudDescriptor.NUM_SHARDS),
Map.entry(CoreAdminParams.REPLICA_TYPE, CloudDescriptor.REPLICA_TYPE));

private static Map<String, CoreAdminOp> initializeOpMap() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
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.CoreAdminParams.ACTION;
import static org.apache.solr.common.params.CoreAdminParams.ROLES;
import static org.apache.solr.handler.admin.CoreAdminHandler.paramToProp;
import static org.apache.solr.security.PermissionNameProvider.Name.CORE_EDIT_PERM;

Expand All @@ -31,7 +30,6 @@
import java.nio.file.Path;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import org.apache.solr.client.api.endpoint.CoreApis;
import org.apache.solr.client.api.model.CreateCoreParams;
Expand All @@ -40,7 +38,6 @@
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.CollectionUtil;
import org.apache.solr.common.util.PropertiesUtil;
import org.apache.solr.common.util.StrUtils;
import org.apache.solr.common.util.Utils;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.handler.admin.CoreAdminHandler;
Expand Down Expand Up @@ -138,11 +135,6 @@ private Map<String, String> buildCoreParams(CreateCoreParams createParams) {
if (createParamMap.containsKey(entry.getKey())) {
Object value = createParamMap.get(entry.getKey());
if (value != null) {
if (entry.getKey().equals(ROLES) && value instanceof List) {
@SuppressWarnings("unchecked")
final var values = (List<String>) value;
value = StrUtils.join(values, ',');
}
coreParams.put(entry.getValue(), value.toString());
}
}
Expand Down Expand Up @@ -185,9 +177,6 @@ public static CreateCoreParams createRequestBodyFromV1Params(SolrParams solrPara
v1ParamMap.remove(key);
collectionProperties.put(key.substring(COLLECTION_PARAM_PREFIX.length()), value.toString());
}
if (key.equals(ROLES)) {
v1ParamMap.put(ROLES, value.toString().split(","));
}
}
if (CollectionUtil.isNotEmpty(coreProperties)) {
v1ParamMap.put("properties", coreProperties);
Expand Down
20 changes: 0 additions & 20 deletions solr/core/src/test/org/apache/solr/cloud/OverseerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -811,8 +811,6 @@ public void testStateChange() throws Exception {
"core1",
ZkStateReader.CORE_NODE_NAME_PROP,
"core_node1",
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.RECOVERING.toString());

Expand All @@ -834,8 +832,6 @@ public void testStateChange() throws Exception {
"shard1",
ZkStateReader.CORE_NAME_PROP,
"core1",
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.ACTIVE.toString());

Expand Down Expand Up @@ -1598,8 +1594,6 @@ public void testReplay() throws Exception {
COLLECTION,
ZkStateReader.CORE_NAME_PROP,
"core1",
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.RECOVERING.toString());
queue.offer(m);
Expand All @@ -1615,8 +1609,6 @@ public void testReplay() throws Exception {
COLLECTION,
ZkStateReader.CORE_NAME_PROP,
"core2",
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.RECOVERING.toString());
queue.offer(m);
Expand All @@ -1637,8 +1629,6 @@ public void testReplay() throws Exception {
COLLECTION,
ZkStateReader.CORE_NAME_PROP,
"core3",
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.RECOVERING.toString());
queue.offer(m);
Expand Down Expand Up @@ -1700,8 +1690,6 @@ public void testExternalClusterStateChangeBehavior() throws Exception {
"core1",
ZkStateReader.CORE_NODE_NAME_PROP,
"core_node1",
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.DOWN.toString());

Expand All @@ -1722,8 +1710,6 @@ public void testExternalClusterStateChangeBehavior() throws Exception {
"c1",
ZkStateReader.CORE_NAME_PROP,
"core1",
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.RECOVERING.toString());

Expand All @@ -1741,8 +1727,6 @@ public void testExternalClusterStateChangeBehavior() throws Exception {
"c1",
ZkStateReader.CORE_NAME_PROP,
"core1",
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.ACTIVE.toString());

Expand Down Expand Up @@ -2015,8 +1999,6 @@ public void testRemovalOfLastReplica() throws Exception {
"core" + N,
ZkStateReader.CORE_NODE_NAME_PROP,
"core_node" + N,
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.RECOVERING.toString());

Expand Down Expand Up @@ -2048,8 +2030,6 @@ public void testRemovalOfLastReplica() throws Exception {
COLLECTION,
ZkStateReader.CORE_NAME_PROP,
"core" + N,
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.ACTIVE.toString());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,6 @@ private void testQueryAgainstDownReplica() throws Exception {
"shard1",
ZkStateReader.CORE_NAME_PROP,
notLeader.getStr(ZkStateReader.CORE_NAME_PROP),
ZkStateReader.ROLES_PROP,
"",
ZkStateReader.STATE_PROP,
Replica.State.DOWN.toString());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public void testCreateCoreRequestBodyMappingAllParams() throws Exception {
v1Params.add("coreNodeName", "someNodeName");
v1Params.add("newCollection", "true");
v1Params.add("async", "someAsyncId");
v1Params.add("roles", "role1,role2");
v1Params.add("collection", "someCollection");
v1Params.add("property.foo", "fooVal");
v1Params.add("property.bar", "barVal");
Expand All @@ -85,9 +84,6 @@ public void testCreateCoreRequestBodyMappingAllParams() throws Exception {
assertEquals(Boolean.TRUE, createRequestBody.isTransient);
assertEquals(Boolean.TRUE, createRequestBody.newCollection);
assertEquals("someNodeName", createRequestBody.coreNodeName);
assertEquals(2, createRequestBody.roles.size());
assertEquals("role1", createRequestBody.roles.get(0));
assertEquals("role2", createRequestBody.roles.get(1));
assertNotNull(createRequestBody.properties);
assertEquals(2, createRequestBody.properties.size());
assertEquals("fooVal", createRequestBody.properties.get("foo"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ public class ZkStateReader implements SolrCloseable {
public static final String BASE_URL_PROP = "base_url";
public static final String NODE_NAME_PROP = "node_name";
public static final String CORE_NODE_NAME_PROP = "core_node_name";
public static final String ROLES_PROP = "roles";
public static final String STATE_PROP = "state";
// if this flag equals to false and the replica does not exist in cluster state, set state op
// become no op (default is true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ public static class Create extends CoreAdminRequest {
protected String collection;
private Integer numShards;
private String shardId;
private String roles;
private String coreNodeName;
private Boolean loadOnStartup;
private Boolean isTransient;
Expand Down Expand Up @@ -103,10 +102,6 @@ public void setShardId(String shardId) {
this.shardId = shardId;
}

public void setRoles(String roles) {
this.roles = roles;
}

public void setCoreNodeName(String coreNodeName) {
this.coreNodeName = coreNodeName;
}
Expand Down Expand Up @@ -156,10 +151,6 @@ public String getShardId() {
return shardId;
}

public String getRoles() {
return roles;
}

public String getCoreNodeName() {
return coreNodeName;
}
Expand Down Expand Up @@ -226,9 +217,6 @@ public SolrParams getParams() {
if (shardId != null) {
params.set(CoreAdminParams.SHARD, shardId);
}
if (roles != null) {
params.set(CoreAdminParams.ROLES, roles);
}
if (coreNodeName != null) {
params.set(CoreAdminParams.CORE_NODE_NAME, coreNodeName);
}
Expand Down
Loading
Loading