You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/10/21 22:09:06 UTC

[GitHub] [solr] gezapeti opened a new pull request, #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation

gezapeti opened a new pull request, #1108:
URL: https://github.com/apache/solr/pull/1108

   …le disk space before splitting shards. Useful with shared file systems like HDFS
   
   https://issues.apache.org/jira/browse/SOLR-14251
   
   # Description
   
   Adding an option to the SPLIT_SHARD_OP command to make it possible to skip free space checking before splitting shards. This is a workaround as the check logic is broken for shared filesystems.
   
   # Solution
   
   The option skips the disk space check as a workaround.
   
   # Tests
   
   None.
   
   # Checklist
   
   - [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.
   - [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 `main` branch.
   - [x] I have run `./gradlew check`.
   - [ ] I have added tests for my changes.
   - [x] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


-- 
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


[GitHub] [solr] risdenk commented on a diff in pull request #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1108:
URL: https://github.com/apache/solr/pull/1108#discussion_r1008090130


##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -193,7 +194,9 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
     RTimerTree t;
     if (ccc.getCoreContainer().getNodeConfig().getMetricsConfig().isEnabled()) {
       // check disk space for shard split
-      if (Boolean.parseBoolean(System.getProperty(SHARDSPLIT_CHECKDISKSPACE_ENABLED, "true"))) {
+      if (!message.getBool(
+          SKIP_FREE_SPACE_CHECK,
+          !Boolean.parseBoolean(System.getProperty(SHARDSPLIT_CHECKDISKSPACE_ENABLED, "true")))) {

Review Comment:
   🎉 sorry @gezapeti for the extra work. I was just going through the lucene-solr PRs and noticed this was hanging out there. Didn't realize it was addressed a different 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-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


[GitHub] [solr] risdenk closed pull request #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation

Posted by GitBox <gi...@apache.org>.
risdenk closed pull request #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation
URL: https://github.com/apache/solr/pull/1108


-- 
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


[GitHub] [solr] gezapeti commented on a diff in pull request #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation

Posted by GitBox <gi...@apache.org>.
gezapeti commented on code in PR #1108:
URL: https://github.com/apache/solr/pull/1108#discussion_r1008088536


##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -193,7 +194,9 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
     RTimerTree t;
     if (ccc.getCoreContainer().getNodeConfig().getMetricsConfig().isEnabled()) {
       // check disk space for shard split
-      if (Boolean.parseBoolean(System.getProperty(SHARDSPLIT_CHECKDISKSPACE_ENABLED, "true"))) {
+      if (!message.getBool(
+          SKIP_FREE_SPACE_CHECK,
+          !Boolean.parseBoolean(System.getProperty(SHARDSPLIT_CHECKDISKSPACE_ENABLED, "true")))) {

Review Comment:
   It looks like it's already turned off automatically on HDFS: https://github.com/apache/solr/blob/main/solr/modules/hdfs/src/java/org/apache/solr/hdfs/HdfsDirectoryFactory.java#L196
   
   This can be closed then, I guess :)



-- 
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


[GitHub] [solr] risdenk commented on a diff in pull request #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation

Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #1108:
URL: https://github.com/apache/solr/pull/1108#discussion_r1008064852


##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -193,7 +194,9 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
     RTimerTree t;
     if (ccc.getCoreContainer().getNodeConfig().getMetricsConfig().isEnabled()) {
       // check disk space for shard split
-      if (Boolean.parseBoolean(System.getProperty(SHARDSPLIT_CHECKDISKSPACE_ENABLED, "true"))) {
+      if (!message.getBool(
+          SKIP_FREE_SPACE_CHECK,
+          !Boolean.parseBoolean(System.getProperty(SHARDSPLIT_CHECKDISKSPACE_ENABLED, "true")))) {

Review Comment:
   Do we need both properties? Doesn't `SHARDSPLIT_CHECKDISKSPACE_ENABLED` handle the case we care about already?
   
   If you are running on HDFS you could set `SHARDSPLIT_CHECKDISKSPACE_ENABLED` to false?



-- 
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


[GitHub] [solr] dsmiley commented on a diff in pull request #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation

Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1108:
URL: https://github.com/apache/solr/pull/1108#discussion_r1002329395


##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -199,8 +200,17 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
             "SplitShardCmd: verify that there is enough space on disk to create sub-shards for slice: {}",
             parentShardLeader);
         t = timings.sub("checkDiskSpace");
-        checkDiskSpace(
-            collectionName, slice.get(), parentShardLeader, splitMethod, ccc.getSolrCloudManager());
+        boolean skipFreeSpaceCheck = message.getBool(SKIP_FREE_SPACE_CHECK, false);

Review Comment:
   See the System.getProperty thing above?  Let's just combine this into one so that the system property is the default.
   CC @heythm 



-- 
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


[GitHub] [solr] risdenk commented on pull request #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation

Posted by GitBox <gi...@apache.org>.
risdenk commented on PR #1108:
URL: https://github.com/apache/solr/pull/1108#issuecomment-1295027828

   Fixed a different way in SOLR-16353


-- 
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


[GitHub] [solr] gezapeti commented on a diff in pull request #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation

Posted by GitBox <gi...@apache.org>.
gezapeti commented on code in PR #1108:
URL: https://github.com/apache/solr/pull/1108#discussion_r1002375679


##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -199,8 +200,17 @@ public boolean split(ClusterState clusterState, ZkNodeProps message, NamedList<O
             "SplitShardCmd: verify that there is enough space on disk to create sub-shards for slice: {}",
             parentShardLeader);
         t = timings.sub("checkDiskSpace");
-        checkDiskSpace(
-            collectionName, slice.get(), parentShardLeader, splitMethod, ccc.getSolrCloudManager());
+        boolean skipFreeSpaceCheck = message.getBool(SKIP_FREE_SPACE_CHECK, false);

Review Comment:
   Oh, I haven't seen the other check, I've just applied my earlier changes. Now I wonder if this per-call granularity is necessary in this logic. I can see the advantage that retrying a shard split with this error does not require a restart with this parameter.



-- 
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


[GitHub] [solr] dsmiley commented on pull request #1108: SOLR-14251 Add option skipFreeSpaceCheck to split shard operation

Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #1108:
URL: https://github.com/apache/solr/pull/1108#issuecomment-1295364124

   @risdenk, @gezapeti is already aware; this is complementary.  Ideally you could easily globally disable it, but maybe there is a scenario for toggling on a specific request (personally I don't care about the latter; maybe @gezapeti does).
   
   Meanwhile, my colleague @heythm is working on a new Solr utility that would try to make settings like this more regularized so we have it both ways all the time (set via request, or system property, or env variable, or maybe other ways).


-- 
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