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/12/07 10:31:37 UTC
[GitHub] [solr] epugh opened a new pull request, #1217: SOLR-10461: move setAliveCheckInterval from clients into Builder
epugh opened a new pull request, #1217:
URL: https://github.com/apache/solr/pull/1217
https://issues.apache.org/jira/browse/SOLR-10461
# Description
Move to using builders everywhere.
# Solution
move setAliveCheckInterval from client to Builder.
# Tests
Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.
# Checklist
Please review the following and check all that apply:
- [ ] 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.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] 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)
- [ ] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] 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] epugh commented on pull request #1217: SOLR-10461: move setAliveCheckInterval from clients into Builder
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1217:
URL: https://github.com/apache/solr/pull/1217#issuecomment-1353655272
> needs CHANGES.txt too
I will add this at the end, I find I always get merge conflicts when I add this early...
--
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 #1217: SOLR-10461: move setAliveCheckInterval from clients into Builder
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1217:
URL: https://github.com/apache/solr/pull/1217#discussion_r1043792886
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java:
##########
@@ -301,14 +301,33 @@ public void onFailure(Throwable oe) {
public static class Builder {
private final Http2SolrClient http2Client;
private final String[] baseSolrUrls;
+ private int interval;
Review Comment:
aliveCheckInterval would be clearer!
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java:
##########
@@ -301,14 +301,33 @@ public void onFailure(Throwable oe) {
public static class Builder {
private final Http2SolrClient http2Client;
private final String[] baseSolrUrls;
+ private int interval;
public Builder(Http2SolrClient http2Client, String... baseSolrUrls) {
this.http2Client = http2Client;
this.baseSolrUrls = baseSolrUrls;
}
+ /**
+ * LBHttpSolrServer keeps pinging the dead servers at fixed interval to find if it is alive. Use
+ * this to set that interval
+ *
+ * @param interval time in milliseconds
+ */
+ public LBHttp2SolrClient.Builder setAliveCheckInterval(int interval) {
Review Comment:
use "with" builder naming convention
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java:
##########
@@ -259,6 +261,21 @@ public Builder withBaseSolrUrls(String... solrUrls) {
return this;
}
+ /**
+ * LBHttpSolrServer keeps pinging the dead servers at fixed interval to find if it is alive. Use
+ * this to set that interval
+ *
+ * @param interval time in milliseconds
+ */
+ public Builder setAliveCheckInterval(int interval) {
Review Comment:
use "with" builder naming convention
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java:
##########
@@ -301,14 +301,33 @@ public void onFailure(Throwable oe) {
public static class Builder {
private final Http2SolrClient http2Client;
private final String[] baseSolrUrls;
+ private int interval;
Review Comment:
On the other hand, this merely uses the very same field name used in the client so... let's either change them all or leave them all (as you wish).
--
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] epugh commented on pull request #1217: SOLR-10461: move setAliveCheckInterval from clients into Builder
Posted by GitBox <gi...@apache.org>.
epugh commented on PR #1217:
URL: https://github.com/apache/solr/pull/1217#issuecomment-1353674604
> needs CHANGES.txt too
Went ahad and added it. Ugh, got conflicts.
--
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] epugh merged pull request #1217: SOLR-10461: move setAliveCheckInterval from clients into Builder
Posted by GitBox <gi...@apache.org>.
epugh merged PR #1217:
URL: https://github.com/apache/solr/pull/1217
--
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
Re: [PR] SOLR-10461: move setAliveCheckInterval from clients into Builder [solr]
Posted by "mtchorz (via GitHub)" <gi...@apache.org>.
mtchorz commented on PR #1217:
URL: https://github.com/apache/solr/pull/1217#issuecomment-1866724918
@epugh After deprecating [setAliveCheckInterval](https://github.com/apache/solr/blob/releases/solr/9.4.0/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java#L450), how I can set aliveCheckInterval when I am using CloudHttp2SolrClient ?
CloudHttp2SolrClient builder does not allow to create LBHttp2SolrClient.
--
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] epugh commented on a diff in pull request #1217: SOLR-10461: move setAliveCheckInterval from clients into Builder
Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1217:
URL: https://github.com/apache/solr/pull/1217#discussion_r1050102202
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java:
##########
@@ -259,6 +261,21 @@ public Builder withBaseSolrUrls(String... solrUrls) {
return this;
}
+ /**
+ * LBHttpSolrServer keeps pinging the dead servers at fixed interval to find if it is alive. Use
+ * this to set that interval
+ *
+ * @param interval time in milliseconds
+ */
+ public Builder setAliveCheckInterval(int interval) {
Review Comment:
I want to fix this as part of SOLR-16590 to get teh same name everywhere...
--
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] stillalex commented on a diff in pull request #1217: SOLR-10461: move setAliveCheckInterval from clients into Builder
Posted by GitBox <gi...@apache.org>.
stillalex commented on code in PR #1217:
URL: https://github.com/apache/solr/pull/1217#discussion_r1056829119
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java:
##########
@@ -181,6 +182,7 @@ public HttpClient getHttpClient() {
public static class Builder extends SolrClientBuilder<Builder> {
protected final List<String> baseSolrUrls;
protected HttpSolrClient.Builder httpSolrClientBuilder;
+ private int aliveCheckInterval;
Review Comment:
@epugh this should be init to the default CHECK_INTERVAL same as below https://github.com/apache/solr/pull/1217/files#diff-8a5ab47986f979f1134864e1352f6617e8d425f756018da7e0c1fa191fcc8fafR81
--
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] stillalex commented on a diff in pull request #1217: SOLR-10461: move setAliveCheckInterval from clients into Builder
Posted by GitBox <gi...@apache.org>.
stillalex commented on code in PR #1217:
URL: https://github.com/apache/solr/pull/1217#discussion_r1056829264
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttp2SolrClient.java:
##########
@@ -301,14 +301,33 @@ public void onFailure(Throwable oe) {
public static class Builder {
private final Http2SolrClient http2Client;
private final String[] baseSolrUrls;
+ private int aliveCheckInterval;
Review Comment:
same for this one. default should be CHECK_INTERVAL
--
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