Re: [PR] SOLR-14115: Allow bin/solr zk commands to have parity with zkcli.sh commands. [solr]
epugh commented on PR #2298: URL: https://github.com/apache/solr/pull/2298#issuecomment-1974971954 Making progress...I *think* I have `bin/solr` done, and I am *hoping* I got `bin/solr.cmd` done... Now looking at `cloud.sh` cc @gus-asf on these changes coming up -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Fix getting fieldType by its name in FileBasedSpellChecker [solr]
epugh commented on code in PR #2329: URL: https://github.com/apache/solr/pull/2329#discussion_r1510124245 ## solr/CHANGES.txt: ## @@ -27,7 +27,8 @@ Optimizations Bug Fixes - -(No changes) + +* PR#2329: Fix getting fieldType by its name in FileBasedSpellChecker (Andrey Bozhko) Review Comment: i confirmed that yes, please open a jira... -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16505: Switch UpdateShardHandler.getRecoveryOnlyHttpClient to Jetty HTTP2 [solr]
dsmiley commented on code in PR #2276: URL: https://github.com/apache/solr/pull/2276#discussion_r1510020415 ## solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java: ## @@ -915,18 +907,15 @@ private final void sendPrepRecoveryCmd(String leaderBaseUrl, String leaderCoreNa int readTimeout = conflictWaitMs + Integer.parseInt(System.getProperty("prepRecoveryReadTimeoutExtraWait", "8000")); Review Comment: ooh, okay, here we have a special timeout option unique to this method. ## solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java: ## @@ -175,24 +175,21 @@ public final void setRecoveringAfterStartup(boolean recoveringAfterStartup) { this.recoveringAfterStartup = recoveringAfterStartup; } - /** Builds a new HttpSolrClient for use in recovery. Caller must close */ - private HttpSolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { + private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { // workaround for SOLR-13605: get the configured timeouts & set them directly // (even though getRecoveryOnlyHttpClient() already has them set) final UpdateShardHandlerConfig cfg = cc.getConfig().getUpdateShardHandlerConfig(); -return (new HttpSolrClient.Builder(baseUrl) +return new Http2SolrClient.Builder(baseUrl) .withDefaultCollection(leaderCoreName) -.withConnectionTimeout(cfg.getDistributedConnectionTimeout(), TimeUnit.MILLISECONDS) -.withSocketTimeout(cfg.getDistributedSocketTimeout(), TimeUnit.MILLISECONDS) - .withHttpClient(cc.getUpdateShardHandler().getRecoveryOnlyHttpClient())); + .withHttpClient(cc.getUpdateShardHandler().getRecoveryOnlyHttpClient()); Review Comment: FYI when re-using a common SolrClient for requests that may need to go to various URLs, use `solrRequest.setBasePath(leaderUrl)` to have the request indicate where it needs to go, for example. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16505: Switch UpdateShardHandler.getRecoveryOnlyHttpClient to Jetty HTTP2 [solr]
dsmiley commented on code in PR #2276: URL: https://github.com/apache/solr/pull/2276#discussion_r1510019464 ## solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java: ## @@ -175,24 +175,21 @@ public final void setRecoveringAfterStartup(boolean recoveringAfterStartup) { this.recoveringAfterStartup = recoveringAfterStartup; } - /** Builds a new HttpSolrClient for use in recovery. Caller must close */ - private HttpSolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { + private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { // workaround for SOLR-13605: get the configured timeouts & set them directly // (even though getRecoveryOnlyHttpClient() already has them set) final UpdateShardHandlerConfig cfg = cc.getConfig().getUpdateShardHandlerConfig(); -return (new HttpSolrClient.Builder(baseUrl) +return new Http2SolrClient.Builder(baseUrl) .withDefaultCollection(leaderCoreName) -.withConnectionTimeout(cfg.getDistributedConnectionTimeout(), TimeUnit.MILLISECONDS) -.withSocketTimeout(cfg.getDistributedSocketTimeout(), TimeUnit.MILLISECONDS) - .withHttpClient(cc.getUpdateShardHandler().getRecoveryOnlyHttpClient())); + .withHttpClient(cc.getUpdateShardHandler().getRecoveryOnlyHttpClient()); } // make sure any threads stop retrying @Override public final void close() { close = true; if (prevSendPreRecoveryHttpUriRequest != null) { - prevSendPreRecoveryHttpUriRequest.abort(); + prevSendPreRecoveryHttpUriRequest.cancel(true); Review Comment: ofNullable here too? -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-16505: Switch UpdateShardHandler.getRecoveryOnlyHttpClient to Jetty HTTP2 [solr]
dsmiley commented on code in PR #2276: URL: https://github.com/apache/solr/pull/2276#discussion_r1510019364 ## solr/core/src/java/org/apache/solr/cloud/RecoveryStrategy.java: ## @@ -175,24 +175,21 @@ public final void setRecoveringAfterStartup(boolean recoveringAfterStartup) { this.recoveringAfterStartup = recoveringAfterStartup; } - /** Builds a new HttpSolrClient for use in recovery. Caller must close */ - private HttpSolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { + private Http2SolrClient.Builder recoverySolrClientBuilder(String baseUrl, String leaderCoreName) { // workaround for SOLR-13605: get the configured timeouts & set them directly // (even though getRecoveryOnlyHttpClient() already has them set) final UpdateShardHandlerConfig cfg = cc.getConfig().getUpdateShardHandlerConfig(); -return (new HttpSolrClient.Builder(baseUrl) +return new Http2SolrClient.Builder(baseUrl) .withDefaultCollection(leaderCoreName) -.withConnectionTimeout(cfg.getDistributedConnectionTimeout(), TimeUnit.MILLISECONDS) -.withSocketTimeout(cfg.getDistributedSocketTimeout(), TimeUnit.MILLISECONDS) - .withHttpClient(cc.getUpdateShardHandler().getRecoveryOnlyHttpClient())); + .withHttpClient(cc.getUpdateShardHandler().getRecoveryOnlyHttpClient()); Review Comment: Ah; now I see we can remove the obsolete comment, because you removed the explicit calls for timeout configuration that shouldn't be necessary (because they should be set on recoveryOnlyHttpClient). Moreover, to my point earlier, we shouldn't need to be creating a new Http2SolrClient at all though! Thus `recoverySolrClientBuilder` could disappear. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Jira/solr 16567 tentative [solr]
dsmiley commented on PR #1246: URL: https://github.com/apache/solr/pull/1246#issuecomment-1974872456 I suppose this should be closed; you were merely showing me something? I suppose this approach of showing somebody something can also be done on one's personal fork as well; probably better. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] SOLR-17190: Replace org.apache.solr.util.LongSet with hppc LongHashSet [solr]
dsmiley commented on PR #2328: URL: https://github.com/apache/solr/pull/2328#issuecomment-1974871824 Just remove it; it's an internal utility with alternatives. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Tests: JettyConfig.Builder.setContext("/solr") [solr]
dsmiley merged PR #2331: URL: https://github.com/apache/solr/pull/2331 -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Fix getting fieldType by its name in FileBasedSpellChecker [solr]
epugh commented on code in PR #2329: URL: https://github.com/apache/solr/pull/2329#discussion_r1510001348 ## solr/CHANGES.txt: ## @@ -27,7 +27,8 @@ Optimizations Bug Fixes - -(No changes) + +* PR#2329: Fix getting fieldType by its name in FileBasedSpellChecker (Andrey Bozhko) Review Comment: I *believe* we normally create a JIRA for these user facing, so we can track which version of Solr it gets released. The pattern has recently become a bit muddier... non user facing changes sometimes skipping JIRA.. If you add a JIRA, then that makes it easier for me to merge this! ## solr/core/src/test-files/solr/collection1/conf/schema-spellchecker.xml: ## @@ -49,9 +49,22 @@ + Review Comment: I haven't checked in more detail, but don't we normally camelCase? testStopType ??? -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Add DateRangeField example to Ref Guide [solr]
epugh merged PR #2330: URL: https://github.com/apache/solr/pull/2330 -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Add DateRangeField example to Ref Guide [solr]
epugh commented on PR #2330: URL: https://github.com/apache/solr/pull/2330#issuecomment-1974841764 Thank you @AndreyBozhko for following up from the other issue. I bumped the example up to give it a bit more prominance. Keep the improvements coming. In the future, I would love to rework the Ref Guide to seperate out the Reference Materials from the How To guides.. So what you added would be a great example of the content that would go into a How To 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Add DateRangeField example to Ref Guide [solr]
epugh commented on code in PR #2330: URL: https://github.com/apache/solr/pull/2330#discussion_r1509997419 ## solr/solr-ref-guide/modules/indexing-guide/pages/date-formatting-math.adoc: ## @@ -217,4 +217,117 @@ Unlike most local params, `op` is actually _not_ defined by any query parser (`f In the above example, it would find documents with indexed ranges that _contain_ (or equals) the range 2013 thru 2018. Multi-valued overlapping indexed ranges in a document are effectively coalesced. -For a DateRangeField example use-case, see https://cwiki.apache.org/confluence/display/solr/DateRangeField[Solr's community wiki]. +=== DateRangeField Example Use Case + +Suppose we want to find all restaurants that are open within a certain time window. +Let's add a date range field to the schema.xml, so that we could index the information about the restaurant opening hours: + +[source,xml] + + + + + +Next, we will add two restaurants to the index: + + +[.tab-label]*JSON* +[source,json] + +[{ "id": "r01", + "opening_hours": [ "[2016-02-01T03:00Z TO 2016-02-01T15:00Z]", + "[2016-02-02T03:00Z TO 2016-02-02T15:00Z]", + "[2016-02-03T03:00Z TO 2016-02-03T15:00Z]", + "[2016-02-04T03:00Z TO 2016-02-04T15:00Z]", + "[2016-02-05T03:00Z TO 2016-02-05T16:00Z]", + "[2016-02-06T03:00Z TO 2016-02-06T16:00Z]", + "[2016-02-07T03:00Z TO 2016-02-07T15:00Z]" ]}, + { "id": "r02", + "opening_hours": [ "[2016-02-06T10:00Z TO 2016-02-06T12:00Z]", + "[2016-02-06T14:00Z TO 2016-02-06T16:00Z]", + "[2016-02-07T12:00Z TO 2016-02-07T16:00Z]" ]} +] + + + +Each restaurant can have multiple opening hours in a single day, +and the opening hours can be different on different days. + +NOTE: The date ranges in opening_hours should be converted to UTC before indexing. Review Comment: maybe back ticks around `opening_hours`? ## solr/solr-ref-guide/modules/query-guide/pages/learning-to-rank.adoc: ## @@ -811,8 +811,8 @@ Related links: * {solr-javadocs}/modules/ltr/org/apache/solr/ltr/feature/Feature.html[Feature javadocs] * {solr-javadocs}/modules/ltr/org/apache/solr/ltr/norm/Normalizer.html[Normalizer javadocs] * {solr-javadocs}/modules/ltr/org/apache/solr/ltr/interleaving/Interleaving.html[Interleaving javadocs] -* https://cwiki.apache.org/confluence/display/solr/HowToContribute -* https://cwiki.apache.org/confluence/display/LUCENE/HowToContribute +* https://github.com/apache/solr/blob/main/CONTRIBUTING.md[Contributing to Solr] Review Comment: i wasnt' sure about having these links at all, but in reading through the page, now i see, makes sense. thanks for fixing. -- 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
Re: [PR] Add DateRangeField example to Ref Guide [solr]
epugh commented on code in PR #2330: URL: https://github.com/apache/solr/pull/2330#discussion_r1509997243 ## solr/core/src/test/org/apache/solr/request/macro/TestMacroExpander.java: ## @@ -37,8 +37,7 @@ public void testExamples() { me = new MacroExpander(testParams, failOnMissingParams); } -// default examples: https://cwiki.apache.org/confluence/display/solr/Parameter+Substitution -// and http://yonik.com/solr-query-parameter-substitution/ +// default examples: https://yonik.com/solr-query-parameter-substitution/ Review Comment: While this is fine, it does remind me that we should probably have a community discussion about how do we get more blogs that ahve great content integrated (@gerlowskija was pushing on that). And maybe we start going through content and bringing it into the 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-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org