You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/07/16 23:22:28 UTC
[GitHub] [lucene-solr] chatman opened a new pull request #1679: SOLR-14656: Removing autoscaling from master
chatman opened a new pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679
This is a WIP PR for removing autoscaling framework from master. We need this to free up @murblanc and @sigram to start afresh with the new autoscaling framework design and implementation from a clean slate.
(This PR is meant only for master. The deprecation on 8x will be done via a separate PR.)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] chatman commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r457248675
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
##########
@@ -446,7 +444,6 @@ public static Modify modifyCollection(String collection, Map<String, Object> pro
protected Boolean autoAddReplicas;
protected String alias;
protected String[] rule , snitch;
- protected String withCollection;
Review comment:
Removed withCollection support. We can see if this can be implemented in an elegant way going forward. Also, with XCJF thing, this may now be irrelevant.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] chatman commented on pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#issuecomment-659955211
This PR now removes the autoscaling framework, including the policy engine etc. It also removes the sim framework since it was heavily intertwined with the autoscaling framework. It only retains the LegacyAssignStrategy and RulesAssignStrategy. I shall raise another PR for the ref guide changes.
This PR brings down the number of tests from 917 to 866.
@sigram @murblanc @noblepaul please review.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r456851602
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/ShardSplitTest.java
##########
@@ -635,7 +635,7 @@ private void doSplitShardWithRule(SolrIndexSplitter.SplitMethod splitMethod) thr
log.info("Starting testSplitShardWithRule");
String collectionName = "shardSplitWithRule_" + splitMethod.toLower();
CollectionAdminRequest.Create createRequest = CollectionAdminRequest.createCollection(collectionName, "conf1", 1, 2)
- .setRule("shard:*,replica:<2,node:*");
Review comment:
Yeah , true . it was the previous rule
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul removed a comment on pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
noblepaul removed a comment on pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#issuecomment-660597516
LGTM
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] chatman commented on pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#issuecomment-660937670
@murblanc I think this is ready for another review. I'm still working on the last piece: removing the suggestions tab in UI.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r456442024
##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -663,17 +553,14 @@ public AssignStrategyFactory(SolrCloudManager solrCloudManager) {
public AssignStrategy create(ClusterState clusterState, DocCollection collection) throws IOException, InterruptedException {
@SuppressWarnings({"unchecked", "rawtypes"})
List<Map> ruleMaps = (List<Map>) collection.get("rule");
- String policyName = collection.getStr(POLICY);
@SuppressWarnings({"rawtypes"})
List snitches = (List) collection.get(SNITCH);
Strategy strategy = null;
- if ((ruleMaps == null || ruleMaps.isEmpty()) && !usePolicyFramework(collection, solrCloudManager)) {
+ if ((ruleMaps == null || ruleMaps.isEmpty())) {
strategy = Strategy.LEGACY;
} else if (ruleMaps != null && !ruleMaps.isEmpty()) {
Review comment:
This if clause if the opposite of the previous one, so basically it's just an else. I'd keep this one as the first tested and let the else pick LEGACY.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#issuecomment-660597516
LGTM
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r456358283
##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java
##########
@@ -148,17 +146,14 @@ public void call(ClusterState state, ZkNodeProps message, @SuppressWarnings({"ra
}
}
- AtomicReference<PolicyHelper.SessionWrapper> sessionWrapper = new AtomicReference<>();
List<CreateReplica> createReplicas;
try {
Review comment:
minor: remove try bloc if no finally
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] chatman commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r456133302
##########
File path: solr/core/src/test/org/apache/solr/cloud/MetricsHistoryIntegrationTest.java
##########
@@ -59,38 +59,24 @@
@BeforeClass
public static void setupCluster() throws Exception {
- boolean simulated = TEST_NIGHTLY ? random().nextBoolean() : true;
- if (simulated) {
- cloudManager = SimCloudManager.createCluster(1, TimeSource.get("simTime:50"));
- solrClient = ((SimCloudManager)cloudManager).simGetSolrClient();
- }
configureCluster(1)
.addConfig("conf", configset("cloud-minimal"))
.configure();
- if (!simulated) {
- cloudManager = cluster.getJettySolrRunner(0).getCoreContainer().getZkController().getSolrCloudManager();
- solrClient = cluster.getSolrClient();
- }
+ cloudManager = cluster.getJettySolrRunner(0).getCoreContainer().getZkController().getSolrCloudManager();
+ solrClient = cluster.getSolrClient();
timeSource = cloudManager.getTimeSource();
// create .system
CollectionAdminRequest.createCollection(CollectionAdminParams.SYSTEM_COLL, null, 1, 1)
.process(solrClient);
CloudUtil.waitForState(cloudManager, CollectionAdminParams.SYSTEM_COLL,
30, TimeUnit.SECONDS, CloudUtil.clusterShape(1, 1));
solrClient.query(CollectionAdminParams.SYSTEM_COLL, params(CommonParams.Q, "*:*"));
- // sleep a little to allow the handler to collect some metrics
- if (simulated) {
- timeSource.sleep(90000);
- } else {
- timeSource.sleep(100000);
- }
+ // sleep until next generation of kids grow up to allow the handler to collect some metrics
Review comment:
@markrmiller @noblepaul any thoughts here, please?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r456449672
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/ShardSplitTest.java
##########
@@ -635,7 +635,7 @@ private void doSplitShardWithRule(SolrIndexSplitter.SplitMethod splitMethod) thr
log.info("Starting testSplitShardWithRule");
String collectionName = "shardSplitWithRule_" + splitMethod.toLower();
CollectionAdminRequest.Create createRequest = CollectionAdminRequest.createCollection(collectionName, "conf1", 1, 2)
- .setRule("shard:*,replica:<2,node:*");
Review comment:
Doesn't seem to be an Autoscaling related rule.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r456449916
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/ShardSplitTest.java
##########
@@ -635,7 +635,7 @@ private void doSplitShardWithRule(SolrIndexSplitter.SplitMethod splitMethod) thr
log.info("Starting testSplitShardWithRule");
String collectionName = "shardSplitWithRule_" + splitMethod.toLower();
CollectionAdminRequest.Create createRequest = CollectionAdminRequest.createCollection(collectionName, "conf1", 1, 2)
- .setRule("shard:*,replica:<2,node:*");
Review comment:
(whatever that would mean BTW :))
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] chatman closed pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
chatman closed pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] chatman commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r457247758
##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -663,17 +553,14 @@ public AssignStrategyFactory(SolrCloudManager solrCloudManager) {
public AssignStrategy create(ClusterState clusterState, DocCollection collection) throws IOException, InterruptedException {
@SuppressWarnings({"unchecked", "rawtypes"})
List<Map> ruleMaps = (List<Map>) collection.get("rule");
- String policyName = collection.getStr(POLICY);
@SuppressWarnings({"rawtypes"})
List snitches = (List) collection.get(SNITCH);
Strategy strategy = null;
- if ((ruleMaps == null || ruleMaps.isEmpty()) && !usePolicyFramework(collection, solrCloudManager)) {
+ if ((ruleMaps == null || ruleMaps.isEmpty())) {
strategy = Strategy.LEGACY;
} else if (ruleMaps != null && !ruleMaps.isEmpty()) {
Review comment:
Fixed, thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r456447471
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
##########
@@ -446,7 +444,6 @@ public static Modify modifyCollection(String collection, Map<String, Object> pro
protected Boolean autoAddReplicas;
protected String alias;
protected String[] rule , snitch;
- protected String withCollection;
Review comment:
Do we remove the notion of `withCollection` with the Autoscaling framework?
I would prefer to let the client manage it manually (i.e. specify its replica placement carefully) if there's a co-location need, but if we keep it we need to keep it here as well I believe, and if we remove it there are many other places from which it should be removed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] chatman commented on pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#issuecomment-660992518
Merged, thanks @noblepaul @murblanc.
AFAIK, all traces of autoscaling have been removed. However, if there's something still left over, we can remove them in another PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] murblanc commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r456442024
##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -663,17 +553,14 @@ public AssignStrategyFactory(SolrCloudManager solrCloudManager) {
public AssignStrategy create(ClusterState clusterState, DocCollection collection) throws IOException, InterruptedException {
@SuppressWarnings({"unchecked", "rawtypes"})
List<Map> ruleMaps = (List<Map>) collection.get("rule");
- String policyName = collection.getStr(POLICY);
@SuppressWarnings({"rawtypes"})
List snitches = (List) collection.get(SNITCH);
Strategy strategy = null;
- if ((ruleMaps == null || ruleMaps.isEmpty()) && !usePolicyFramework(collection, solrCloudManager)) {
+ if ((ruleMaps == null || ruleMaps.isEmpty())) {
strategy = Strategy.LEGACY;
} else if (ruleMaps != null && !ruleMaps.isEmpty()) {
Review comment:
This if clause if the opposite of the previous one (in the new version of the code even though github puts my comment on the old version), so basically it's just an else. I'd keep this one as the first tested and let the else pick LEGACY.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1679: SOLR-14656: Removing autoscaling from master
Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1679:
URL: https://github.com/apache/lucene-solr/pull/1679#discussion_r456857886
##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
##########
@@ -446,7 +444,6 @@ public static Modify modifyCollection(String collection, Map<String, Object> pro
protected Boolean autoAddReplicas;
protected String alias;
protected String[] rule , snitch;
- protected String withCollection;
Review comment:
`withCollection` has made an already complex rule engine even more complex.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org