You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "noblepaul (via GitHub)" <gi...@apache.org> on 2023/04/26 11:29:34 UTC
[GitHub] [solr] noblepaul commented on a diff in pull request #1477: SOLR-16712: Simplify DocCollection ctor for PRS enabled collection
noblepaul commented on code in PR #1477:
URL: https://github.com/apache/solr/pull/1477#discussion_r1150090351
##########
solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java:
##########
@@ -146,6 +143,45 @@ public DocCollection(
assert name != null && slices != null;
}
+ public static DocCollection buildDocCollection(
Review Comment:
just `create()` would be a better name?
##########
solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java:
##########
@@ -155,13 +191,15 @@ public static String getCollectionPathRoot(String coll) {
}
/**
- * Update our state with a state of a {@link Replica} Used to create a new Collection State when
- * only a replica is updated
+ * Update our state with a state of a {@link PerReplicaStates} which could override states of
+ * {@link Replica}.
+ *
+ * <p>This does not create a new DocCollection.
*/
- public DocCollection copyWith(PerReplicaStates newPerReplicaStates) {
- if (this.prsSupplier != null) {
- log.debug("In-place update of PRS: {}", newPerReplicaStates);
- this.prsSupplier.prs = newPerReplicaStates;
+ public final DocCollection setPerReplicaStates(PerReplicaStates perReplicaStates) {
Review Comment:
@patsonluk , can you please address the above ?
##########
solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java:
##########
@@ -155,13 +191,15 @@ public static String getCollectionPathRoot(String coll) {
}
/**
- * Update our state with a state of a {@link Replica} Used to create a new Collection State when
- * only a replica is updated
+ * Update our state with a state of a {@link PerReplicaStates} which could override states of
+ * {@link Replica}.
+ *
+ * <p>This does not create a new DocCollection.
*/
- public DocCollection copyWith(PerReplicaStates newPerReplicaStates) {
- if (this.prsSupplier != null) {
- log.debug("In-place update of PRS: {}", newPerReplicaStates);
- this.prsSupplier.prs = newPerReplicaStates;
+ public final DocCollection setPerReplicaStates(PerReplicaStates perReplicaStates) {
Review Comment:
as it is mutating an existing Object, there is a small period during which the `state` of a given replica is unpredictable . some replicas of the collection would fetch state from old PRS and some will point to new PRS
The question is, what did we achieve with this change?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org