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/08/23 15:38:37 UTC
[GitHub] [solr] heythm opened a new pull request, #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
heythm opened a new pull request, #983:
URL: https://github.com/apache/solr/pull/983
https://issues.apache.org/jira/browse/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] dsmiley merged pull request #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
dsmiley merged PR #983:
URL: https://github.com/apache/solr/pull/983
--
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 #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #983:
URL: https://github.com/apache/solr/pull/983#issuecomment-1228041762
Since it's a known issue that this check doesn't make sense for HDFS, we could document that in the ref guide for an HDFS page.
--
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] sonatype-lift[bot] commented on a diff in pull request #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #983:
URL: https://github.com/apache/solr/pull/983#discussion_r953001426
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -811,6 +811,10 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws SolrException {
+ // check that the system property is enabled. It should not be disabled by default.
+ if (System.getProperty("solr.shardSplit.checkDiskSpace").equals("true")) {
Review Comment:
*NULL_DEREFERENCE:* object returned by `getProperty("solr.shardSplit.checkDiskSpace")` could be null and is dereferenced at line 815.
---
Reply with *"**@sonatype-lift help**"* for info about LiftBot commands.
Reply with *"**@sonatype-lift ignore**"* to tell LiftBot to leave out the above finding from this PR.
Reply with *"**@sonatype-lift ignoreall**"* to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to **refresh** the page to see its response. [Click here](https://help.sonatype.com/lift/talking-to-lift) to get to know more about LiftBot commands.
---
Was this a good recommendation?
[ [🙁 Not relevant](https://www.sonatype.com/lift-comment-rating?comment=320757696&lift_comment_rating=1) ] - [ [😕 Won't fix](https://www.sonatype.com/lift-comment-rating?comment=320757696&lift_comment_rating=2) ] - [ [😑 Not critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=320757696&lift_comment_rating=3) ] - [ [🙂 Critical, will fix](https://www.sonatype.com/lift-comment-rating?comment=320757696&lift_comment_rating=4) ] - [ [😊 Critical, fixing now](https://www.sonatype.com/lift-comment-rating?comment=320757696&lift_comment_rating=5) ]
--
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] heythm commented on a diff in pull request #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
heythm commented on code in PR #983:
URL: https://github.com/apache/solr/pull/983#discussion_r953820597
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -811,6 +811,10 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws SolrException {
+ // check that the system property is enabled. It should not be disabled by default.
+ if (System.getProperty("solr.shardSplit.checkDiskSpace").equals("true")) {
Review Comment:
@sonatype-lift ignore
--
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 #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #983:
URL: https://github.com/apache/solr/pull/983#discussion_r957278757
##########
solr/solr-ref-guide/modules/deployment-guide/pages/solrcloud-shards-indexing.adoc:
##########
@@ -161,6 +161,8 @@ You can delete the old shard at a later time when you're ready.
More details on how to use shard splitting is in the section on the Collection API's xref:shard-management.adoc#splitshard[SPLITSHARD command].
+When splitting a shard, a free disk space check is performed on the local file system of the leader shard. This can be disabled through the `solr.shardSplit.checkDiskSpace.enabled` system property (i.e. `-Dsolr.shardSplit.checkDiskSpace.enabled=false`). This property is disabled by default for xref:solr-on-hdfs.adoc[HDFS].
Review Comment:
I don't think this location is a good spot. It's an overview of a variety of things; not details of how to do something. Just above there is a link to `shard-management.adoc` which is an appropriate place for this detail.
--
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 #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #983:
URL: https://github.com/apache/solr/pull/983#discussion_r956882802
##########
solr/solr-ref-guide/modules/deployment-guide/pages/solr-on-hdfs.adoc:
##########
@@ -246,6 +246,17 @@ s|Required |Default: none
+
The Kerberos principal that Solr should use to authenticate to secure Hadoop; the format of a typical Kerberos V5 principal is: `primary/instance@realm`.
+== Shard splitting settings
Review Comment:
The *only* point of putting this documentation here, on the Hdfs page, would be **if** we would recommend the user to do something about this setting if they are using Hdfs. But you flipped this setting off for HdfsDirectory and so I think it's now misplaced. Instead, I'd specify this over in a shard split page.
And when yhou do, ensure it's clear that this is a *system property*; not a parameter or something.
--
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] heythm commented on a diff in pull request #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
heythm commented on code in PR #983:
URL: https://github.com/apache/solr/pull/983#discussion_r952850401
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -811,6 +811,10 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws SolrException {
+ // check that the system property is enabled. It should not be disabled by default.
+ if (!System.getProperty("solr.shardSplit.checkDiskSpace", "true").equals("true")) {
Review Comment:
looking for a more elegant way to set default value and check
--
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] sonatype-lift[bot] commented on a diff in pull request #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on code in PR #983:
URL: https://github.com/apache/solr/pull/983#discussion_r953820741
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -811,6 +811,10 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws SolrException {
+ // check that the system property is enabled. It should not be disabled by default.
+ if (System.getProperty("solr.shardSplit.checkDiskSpace").equals("true")) {
Review Comment:
I've recorded this as ignored for this pull request.
If you change your mind, just comment `@sonatype-lift unignore`.
--
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] heythm commented on pull request #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
heythm commented on PR #983:
URL: https://github.com/apache/solr/pull/983#issuecomment-1229182353
@dsmiley - About toggling this parameter through client requests: It depends on the use cases it will help. Would it be just to reduce the noise of the warnings or for a real utility? If we do not see other use cases to solve, it could be implemented when they are identified so as not to add a non-useful 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 a diff in pull request #983: SOLR-16353: SplitShardCmd.checkDiskSpace can fail for transient core
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #983:
URL: https://github.com/apache/solr/pull/983#discussion_r956883123
##########
solr/CHANGES.txt:
##########
@@ -66,6 +66,8 @@ Improvements
* SOLR-16323: The Docker image now uses the Solr User ID instead of the User Name, helps with non-root checks (Houston Putman)
+* SOLR-16353: Make SplitShardCmd#checkDiskSpace enableable through a system property. (Haythem Khiri)
Review Comment:
I think *disableable*
--
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