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/22 19:32:41 UTC
[GitHub] [solr] epugh opened a new pull request, #1247: SOLR-10464: setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
epugh opened a new pull request, #1247:
URL: https://github.com/apache/solr/pull/1247
https://issues.apache.org/jira/browse/SOLR-10464
# Description
setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
--
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 #1247: SOLR-10464: setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1247:
URL: https://github.com/apache/solr/pull/1247#discussion_r1058591414
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -257,6 +258,17 @@ public Builder setRetryExpiryTime(int secs) {
return this;
}
+ /**
+ * Sets the cache ttl for DocCollection Objects cached.
+ *
+ * @param timeToLiveSeconds ttl value in seconds
+ */
+ public Builder setCollectionCacheTtl(int timeToLiveSeconds) {
Review Comment:
`with`
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudLegacySolrClient.java:
##########
@@ -87,6 +87,7 @@ protected CloudLegacySolrClient(Builder builder) {
this.stateProvider = builder.stateProvider;
}
this.retryExpiryTime = builder.retryExpiryTime;
+ this.collectionStateCache.timeToLiveMs = builder.timeToLive * 1000L;
Review Comment:
if the builder's timeToLive field were timeToLiveSec then it would be clearer its seconds
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudLegacySolrClient.java:
##########
@@ -270,6 +271,17 @@ public Builder withLBHttpSolrClient(LBHttpSolrClient loadBalancedSolrClient) {
return this;
}
+ /**
+ * Sets the cache ttl for DocCollection Objects cached.
+ *
+ * @param seconds ttl value in seconds
+ */
+ public Builder setCollectionCacheTtl(int seconds) {
Review Comment:
`with`
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudLegacySolrClient.java:
##########
@@ -270,6 +271,17 @@ public Builder withLBHttpSolrClient(LBHttpSolrClient loadBalancedSolrClient) {
return this;
}
+ /**
+ * Sets the cache ttl for DocCollection Objects cached.
+ *
+ * @param seconds ttl value in seconds
+ */
+ public Builder setCollectionCacheTTL(int seconds) {
Review Comment:
Changing them again later leads to more changes. You could raise this now.
--
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 #1247: SOLR-10464: setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
Posted by GitBox <gi...@apache.org>.
dsmiley commented on code in PR #1247:
URL: https://github.com/apache/solr/pull/1247#discussion_r1057006582
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudLegacySolrClient.java:
##########
@@ -270,6 +271,17 @@ public Builder withLBHttpSolrClient(LBHttpSolrClient loadBalancedSolrClient) {
return this;
}
+ /**
+ * Sets the cache ttl for DocCollection Objects cached.
+ *
+ * @param seconds ttl value in seconds
+ */
+ public Builder setCollectionCacheTTL(int seconds) {
Review Comment:
The casing doesn't match the [Google Java Style](https://google.github.io/styleguide/javaguide.html#s5.3-camel-case). Use "Ttl" at the end.
Why not "with" naming?
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -147,6 +147,7 @@ public static class Builder {
private ResponseParser responseParser;
private long retryExpiryTime =
TimeUnit.NANOSECONDS.convert(3, TimeUnit.SECONDS); // 3 seconds or 3 million nanos
+ private int timeToLive = 60;
Review Comment:
The name should reflect the units in a suffix
--
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 #1247: SOLR-10464: setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1247:
URL: https://github.com/apache/solr/pull/1247#discussion_r1059069780
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudLegacySolrClient.java:
##########
@@ -270,6 +271,17 @@ public Builder withLBHttpSolrClient(LBHttpSolrClient loadBalancedSolrClient) {
return this;
}
+ /**
+ * Sets the cache ttl for DocCollection Objects cached.
+ *
+ * @param seconds ttl value in seconds
+ */
+ public Builder setCollectionCacheTtl(int seconds) {
Review Comment:
sent a inquery to the mailing list and a mention on the jira... Let's see if that shakes any opinons loose.... Please do speak 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-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 #1247: SOLR-10464: setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1247:
URL: https://github.com/apache/solr/pull/1247#discussion_r1059069458
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudLegacySolrClient.java:
##########
@@ -87,6 +87,7 @@ protected CloudLegacySolrClient(Builder builder) {
this.stateProvider = builder.stateProvider;
}
this.retryExpiryTime = builder.retryExpiryTime;
+ this.collectionStateCache.timeToLiveMs = builder.timeToLive * 1000L;
Review Comment:
Done!
--
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 #1247: SOLR-10464: setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1247:
URL: https://github.com/apache/solr/pull/1247#discussion_r1057972873
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudLegacySolrClient.java:
##########
@@ -270,6 +271,17 @@ public Builder withLBHttpSolrClient(LBHttpSolrClient loadBalancedSolrClient) {
return this;
}
+ /**
+ * Sets the cache ttl for DocCollection Objects cached.
+ *
+ * @param seconds ttl value in seconds
+ */
+ public Builder setCollectionCacheTTL(int seconds) {
Review Comment:
I want to go through and change the namings for all the builders at once, after i send a email to dev@ mailing list to get consensus. As far as the the TTL, I didn't know about the Google Java Style, and so `TTl` looked werid to me compared to `TTL`. I wish one of our checkers had actually flagged that I wasn't following the properly Google Java Style!
--
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 #1247: SOLR-10464: setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1247:
URL: https://github.com/apache/solr/pull/1247#discussion_r1057977092
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -147,6 +147,7 @@ public static class Builder {
private ResponseParser responseParser;
private long retryExpiryTime =
TimeUnit.NANOSECONDS.convert(3, TimeUnit.SECONDS); // 3 seconds or 3 million nanos
+ private int timeToLive = 60;
Review Comment:
Done.... I also renamed a related timeToLive that is in milliseconds to `timeToLiveMs`...
--
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 #1247: SOLR-10464: setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
Posted by GitBox <gi...@apache.org>.
epugh commented on code in PR #1247:
URL: https://github.com/apache/solr/pull/1247#discussion_r1060032483
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudLegacySolrClient.java:
##########
@@ -270,6 +271,17 @@ public Builder withLBHttpSolrClient(LBHttpSolrClient loadBalancedSolrClient) {
return this;
}
+ /**
+ * Sets the cache ttl for DocCollection Objects cached.
+ *
+ * @param seconds ttl value in seconds
+ */
+ public Builder setCollectionCacheTtl(int seconds) {
Review Comment:
I'm going to go with `with` for now, as I'm anxious to get this committed. There is a seperate JIRA out to ask for more global feedback.... and this could always be changed at that point ;-)
--
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 #1247: SOLR-10464: setCollectionCacheTTL should be deprecated in favor of Solr Client Builder methods
Posted by GitBox <gi...@apache.org>.
epugh merged PR #1247:
URL: https://github.com/apache/solr/pull/1247
--
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