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/08/17 22:41:46 UTC
[GitHub] [lucene-solr] adorow opened a new pull request #1759: SOLR-13438: on collection delete, also delete .autocreated config set
adorow opened a new pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Lucene or Solr:
* https://issues.apache.org/jira/projects/LUCENE
* https://issues.apache.org/jira/projects/SOLR
You will need to create an account in Jira in order to create an issue.
The title of the PR should reference the Jira issue number in the form:
* LUCENE-####: <short description of problem or changes>
* SOLR-####: <short description of problem or changes>
LUCENE and SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
On collection delete, also delete its config set iff it has been auto-created and if there are no other references to it.
# Solution
The following steps are followed on (the end of) DeleteCollectionCmd:
* read config set name
* if it is auto-created (suffixed with ".AUTOCREATED"), then:
* verify if any other collection uses the same config set
* if no other collection uses the same config set; then delete it
*Notes*: some comments were added referring to some other codes that do some similar things (like for finding out the configSetName of other collections). Maybe there's a better, reusable way of doing that - like triggering an extra command for deleting the config set, that my limited knowledge in the code has not figured out.
# Tests
Existing tests run successfully.
Added a new test: CollectionDeleteAlsoDeletesAutocreatedConfigSetTest. A collection is created (with it an auto-created config set is added. The collection is deleted, and it is verified that the ".AUTOCREATED" config set has also been deleted.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title. (an issue already existed)
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `master` branch.
- [x] I have run `ant precommit` and the appropriate test suite. (./gradlew check was executed, which seems to be applying the same checks)
- [x] I have added tests for my changes.
- [x] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
----------------------------------------------------------------
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] adorow commented on a change in pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
adorow commented on a change in pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#discussion_r473710901
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
Review comment:
@madrob I will look into that, should be simple to do.
I opted for the separate file initially mainly because that seemed to be the standard for most of the tests I looked into. Is there some specific agreement that I should look into for future reference? Thanks!
**EDIT**: I'm renaming the other test in `SimpleCollectionCreateDeleteTest` from `test` to `testCreateAndDeleteThenCreateAgain`, and naming the new test `testDeleteAlsoDeletesAutocreatedConfigSet`, to better differentiate between them.
----------------------------------------------------------------
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] madrob commented on pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
madrob commented on pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#issuecomment-678409403
I think only committers can retriever the precommit, I launched it again, if it comes back clean will merge
----------------------------------------------------------------
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] adorow commented on pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
adorow commented on pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#issuecomment-678619459
Awesome stuff :) Thanks for the help Mike!
On Fri 21. Aug 2020 at 21:43, Mike Drob <no...@github.com> wrote:
>
>
> Merged #1759 <https://github.com/apache/lucene-solr/pull/1759> into
> master.
>
>
>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/lucene-solr/pull/1759#event-3680906294>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKDRLWMKXFVUBW2PPIVGZDSB3E6FANCNFSM4QCKWTOA>
> .
>
>
>
----------------------------------------------------------------
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] adorow commented on pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
adorow commented on pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#issuecomment-677507729
@madrob one of the GitHub Actions failed because `Could not download commons-text-1.6.jar (org.apache.commons:commons-text:1.6)`. Seems like something that just happened by chance (in the ant build it worked fine). Is there a way to simply run it again?
----------------------------------------------------------------
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] madrob commented on a change in pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#discussion_r473340424
##########
File path: solr/CHANGES.txt
##########
@@ -42,6 +42,8 @@ Improvements
* SOLR-13528 Rate Limiting in Solr (Atri Sharma, Mike Drob)
+* SOLR-13438: On deleting a collection, its config set will also be deleted iff it has been auto-created, and not used by any other collection.
Review comment:
Please include your preferred name here after the change summary so that we can properly credit you!
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
Review comment:
Instead of new file, can this be part of `SimpleCollectionCreateDeleteTest`?
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
+
+ public CollectionDeleteAlsoDeletesAutocreatedConfigSetTest() {
+ sliceCount = 1;
+ }
+
+ @Test
+ @ShardsFixed(num = 1)
+ public void test() throws Exception {
+ String overseerNode = OverseerCollectionConfigSetProcessor.getLeaderNode(cloudClient.getZkStateReader().getZkClient());
+
+ String collectionName = "CollectionDeleteAlsoDeletesAutocreatedConfigSetTest";
+ CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(collectionName,1,1)
+ .setCreateNodeSet(overseerNode);
+
+ NamedList<Object> request = create.process(cloudClient).getResponse();
+
+ if (request.get("success") != null) {
+ // collection exists now
+ assertTrue(cloudClient.getZkStateReader().getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName, false));
+
+ String configName = cloudClient.getZkStateReader().readConfigName(collectionName);
+
+ // config for this collection is '.AUTOCREATED', and exists globally
+ assertTrue(configName.endsWith(".AUTOCREATED"));
+ assertTrue(cloudClient.getZkStateReader().getConfigManager().listConfigs().contains(configName));
+
+ @SuppressWarnings({"rawtypes"})
Review comment:
Please avoid suppressing warnings in new code and use generics
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
+
+ public CollectionDeleteAlsoDeletesAutocreatedConfigSetTest() {
+ sliceCount = 1;
+ }
+
+ @Test
+ @ShardsFixed(num = 1)
+ public void test() throws Exception {
+ String overseerNode = OverseerCollectionConfigSetProcessor.getLeaderNode(cloudClient.getZkStateReader().getZkClient());
+
+ String collectionName = "CollectionDeleteAlsoDeletesAutocreatedConfigSetTest";
+ CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(collectionName,1,1)
+ .setCreateNodeSet(overseerNode);
Review comment:
Do we need this call to `setCreateNodeSet`? If not, can also delete getting overseerNode above. If so, add a comment explaining why.
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
+
+ public CollectionDeleteAlsoDeletesAutocreatedConfigSetTest() {
+ sliceCount = 1;
+ }
+
+ @Test
+ @ShardsFixed(num = 1)
+ public void test() throws Exception {
+ String overseerNode = OverseerCollectionConfigSetProcessor.getLeaderNode(cloudClient.getZkStateReader().getZkClient());
+
+ String collectionName = "CollectionDeleteAlsoDeletesAutocreatedConfigSetTest";
+ CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(collectionName,1,1)
+ .setCreateNodeSet(overseerNode);
+
+ NamedList<Object> request = create.process(cloudClient).getResponse();
+
+ if (request.get("success") != null) {
+ // collection exists now
+ assertTrue(cloudClient.getZkStateReader().getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName, false));
+
+ String configName = cloudClient.getZkStateReader().readConfigName(collectionName);
+
+ // config for this collection is '.AUTOCREATED', and exists globally
+ assertTrue(configName.endsWith(".AUTOCREATED"));
+ assertTrue(cloudClient.getZkStateReader().getConfigManager().listConfigs().contains(configName));
+
+ @SuppressWarnings({"rawtypes"})
+ CollectionAdminRequest delete = CollectionAdminRequest.deleteCollection(collectionName);
Review comment:
Declare as `CollectionAdminRequest.Delete`
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
+
+ public CollectionDeleteAlsoDeletesAutocreatedConfigSetTest() {
+ sliceCount = 1;
+ }
+
+ @Test
+ @ShardsFixed(num = 1)
+ public void test() throws Exception {
Review comment:
Please add another test where we have two collections sharing the same autogenerated configset, and verify deleting one of them doesn't orphan the other.
----------------------------------------------------------------
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] adorow commented on a change in pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
adorow commented on a change in pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#discussion_r473724812
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
+
+ public CollectionDeleteAlsoDeletesAutocreatedConfigSetTest() {
+ sliceCount = 1;
+ }
+
+ @Test
+ @ShardsFixed(num = 1)
+ public void test() throws Exception {
+ String overseerNode = OverseerCollectionConfigSetProcessor.getLeaderNode(cloudClient.getZkStateReader().getZkClient());
+
+ String collectionName = "CollectionDeleteAlsoDeletesAutocreatedConfigSetTest";
+ CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(collectionName,1,1)
+ .setCreateNodeSet(overseerNode);
+
+ NamedList<Object> request = create.process(cloudClient).getResponse();
+
+ if (request.get("success") != null) {
+ // collection exists now
+ assertTrue(cloudClient.getZkStateReader().getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName, false));
+
+ String configName = cloudClient.getZkStateReader().readConfigName(collectionName);
+
+ // config for this collection is '.AUTOCREATED', and exists globally
+ assertTrue(configName.endsWith(".AUTOCREATED"));
+ assertTrue(cloudClient.getZkStateReader().getConfigManager().listConfigs().contains(configName));
+
+ @SuppressWarnings({"rawtypes"})
+ CollectionAdminRequest delete = CollectionAdminRequest.deleteCollection(collectionName);
Review comment:
Since I'm now adding this new scenario into `SimpleCollectionCreateDeleteTest`, I'll apply the same into the other scenario (and therefore also remove `@SuppressWarnings` from that test.
----------------------------------------------------------------
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] adorow commented on a change in pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
adorow commented on a change in pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#discussion_r473727115
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
+
+ public CollectionDeleteAlsoDeletesAutocreatedConfigSetTest() {
+ sliceCount = 1;
+ }
+
+ @Test
+ @ShardsFixed(num = 1)
+ public void test() throws Exception {
+ String overseerNode = OverseerCollectionConfigSetProcessor.getLeaderNode(cloudClient.getZkStateReader().getZkClient());
+
+ String collectionName = "CollectionDeleteAlsoDeletesAutocreatedConfigSetTest";
+ CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(collectionName,1,1)
+ .setCreateNodeSet(overseerNode);
Review comment:
@madrob no other reason before lack of knowledge on my part :) I'm removing that part of the code, tests still work accordingly.
----------------------------------------------------------------
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] adorow commented on a change in pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
adorow commented on a change in pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#discussion_r473727115
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
+
+ public CollectionDeleteAlsoDeletesAutocreatedConfigSetTest() {
+ sliceCount = 1;
+ }
+
+ @Test
+ @ShardsFixed(num = 1)
+ public void test() throws Exception {
+ String overseerNode = OverseerCollectionConfigSetProcessor.getLeaderNode(cloudClient.getZkStateReader().getZkClient());
+
+ String collectionName = "CollectionDeleteAlsoDeletesAutocreatedConfigSetTest";
+ CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(collectionName,1,1)
+ .setCreateNodeSet(overseerNode);
Review comment:
@madrob no other reason besides lack of knowledge on my part :) I'm removing that part of the code, tests still work accordingly.
----------------------------------------------------------------
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] madrob merged pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
madrob merged pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759
----------------------------------------------------------------
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] adorow commented on a change in pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
adorow commented on a change in pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#discussion_r473710901
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionDeleteAlsoDeletesAutocreatedConfigSetTest.java
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.cloud.api.collections;
+
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.cloud.OverseerCollectionConfigSetProcessor;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.NamedList;
+import org.junit.Test;
+
+public class CollectionDeleteAlsoDeletesAutocreatedConfigSetTest extends AbstractFullDistribZkTestBase {
Review comment:
@madrob I will look into that, should be simple to do.
I opted for the separate file initially mainly because that seemed to be the standard for most of the tests I looked into. Is there some specific agreement that I should look into for future reference? 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] madrob edited a comment on pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
madrob edited a comment on pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#issuecomment-678409403
I think only committers can re-launch the precommit, I launched it again, if it comes back clean will merge
----------------------------------------------------------------
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] madrob commented on a change in pull request #1759: SOLR-13438: on collection delete, also delete .AUTOCREATED config set
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1759:
URL: https://github.com/apache/lucene-solr/pull/1759#discussion_r474835389
##########
File path: solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java
##########
@@ -32,69 +28,151 @@
import org.apache.solr.util.TimeOut;
import org.junit.Test;
+import java.util.Collection;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
public class SimpleCollectionCreateDeleteTest extends AbstractFullDistribZkTestBase {
- public SimpleCollectionCreateDeleteTest() {
- sliceCount = 1;
- }
-
- @Test
- @ShardsFixed(num = 1)
- public void test() throws Exception {
- String overseerNode = OverseerCollectionConfigSetProcessor.getLeaderNode(cloudClient.getZkStateReader().getZkClient());
- String notOverseerNode = null;
- for (CloudJettyRunner cloudJetty : cloudJettys) {
- if (!overseerNode.equals(cloudJetty.nodeName)) {
- notOverseerNode = cloudJetty.nodeName;
- break;
- }
+ public SimpleCollectionCreateDeleteTest() {
+ sliceCount = 1;
}
- String collectionName = "SimpleCollectionCreateDeleteTest";
- CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(collectionName,1,1)
- .setCreateNodeSet(overseerNode);
-
- NamedList<Object> request = create.process(cloudClient).getResponse();
-
- if (request.get("success") != null) {
- assertTrue(cloudClient.getZkStateReader().getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName, false));
-
- @SuppressWarnings({"rawtypes"})
- CollectionAdminRequest delete = CollectionAdminRequest.deleteCollection(collectionName);
- cloudClient.request(delete);
-
- assertFalse(cloudClient.getZkStateReader().getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName, false));
-
- // currently, removing a collection does not wait for cores to be unloaded
- TimeOut timeout = new TimeOut(30, TimeUnit.SECONDS, TimeSource.NANO_TIME);
- while (true) {
-
- if( timeout.hasTimedOut() ) {
- throw new TimeoutException("Timed out waiting for all collections to be fully removed.");
+
+ @Test
+ @ShardsFixed(num = 1)
+ public void testCreateAndDeleteThenCreateAgain() throws Exception {
Review comment:
👍
----------------------------------------------------------------
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