You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Sai Sandeep Rangisetti via Review Board <no...@reviews.apache.org> on 2022/10/14 15:08:00 UTC
Review Request 74170: RANGER-3947 fix thread leak in SolrCollectionBootstrapper
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74170/
-----------------------------------------------------------
Review request for ranger.
Bugs: RANGER-3947
https://issues.apache.org/jira/browse/RANGER-3947
Repository: ranger
Description
-------
Closing the solr cloud client in SolrCollectionBootstrapper's retry loop of creating solr config and collection. Without this new solr cloud client is created in every loop and new connection pools which will not be cleaned up and create large number of threads
Diffs
-----
embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java fe4006f76
Diff: https://reviews.apache.org/r/74170/diff/1/
Testing
-------
Ran ranger-admin without ranger_audit config in zk and no contrib/solr_for_audit_setup/conf file which leads to retry loop and verified that threads aren't increasinng
Thanks,
Sai Sandeep Rangisetti
Re: Review Request 74170: RANGER-3947 fix thread leak in SolrCollectionBootstrapper
Posted by Don Bosco Durai <bo...@apache.org>.
> On Oct. 15, 2022, 4:42 p.m., Don Bosco Durai wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java
> > Lines 204 (patched)
> > <https://reviews.apache.org/r/74170/diff/1/?file=2270661#file2270661line204>
> >
> > Should set solrCloudClient to null after close? Also what happens if we set to null, would it get created again?
>
> Sai Sandeep Rangisetti wrote:
> After setting it to null it gets created in the connect method. So I think we don't need to set it to null as it always gets created in the loop
Thanks for clarifying this.
> On Oct. 15, 2022, 4:42 p.m., Don Bosco Durai wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java
> > Lines 207 (patched)
> > <https://reviews.apache.org/r/74170/diff/1/?file=2270661#file2270661line207>
> >
> > 1. Any reason we are setting this as severe? Can we use the level WARN or ERROR?
> > 2. Can we pass the exception as parameter to the logger?
>
> Sai Sandeep Rangisetti wrote:
> 1. This class is using `java.util.logging.Logger` which does not have the level ERROR. It only has severe and warning
> 2. java.util.logging.Logger does not have method to pass exception as parameter in the `logger.warning` or `logger.severe` methods. It has `logger.log(Level, Exception, Supplier)` method but I didn't see it being used in this class so I also didn't use it
Thanks for clarifying this.
- Don Bosco
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74170/#review224798
-----------------------------------------------------------
On Oct. 14, 2022, 3:07 p.m., Sai Sandeep Rangisetti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74170/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2022, 3:07 p.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-3947
> https://issues.apache.org/jira/browse/RANGER-3947
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Closing the solr cloud client in SolrCollectionBootstrapper's retry loop of creating solr config and collection. Without this new solr cloud client is created in every loop and new connection pools which will not be cleaned up and create large number of threads
>
>
> Diffs
> -----
>
> embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java fe4006f76
>
>
> Diff: https://reviews.apache.org/r/74170/diff/1/
>
>
> Testing
> -------
>
> Ran ranger-admin without ranger_audit config in zk and no contrib/solr_for_audit_setup/conf file which leads to retry loop and verified that threads aren't increasinng
>
>
> Thanks,
>
> Sai Sandeep Rangisetti
>
>
Re: Review Request 74170: RANGER-3947 fix thread leak in SolrCollectionBootstrapper
Posted by Sai Sandeep Rangisetti via Review Board <no...@reviews.apache.org>.
> On Oct. 15, 2022, 4:42 p.m., Don Bosco Durai wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java
> > Lines 204 (patched)
> > <https://reviews.apache.org/r/74170/diff/1/?file=2270661#file2270661line204>
> >
> > Should set solrCloudClient to null after close? Also what happens if we set to null, would it get created again?
After setting it to null it gets created in the connect method. So I think we don't need to set it to null as it always gets created in the loop
> On Oct. 15, 2022, 4:42 p.m., Don Bosco Durai wrote:
> > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java
> > Lines 207 (patched)
> > <https://reviews.apache.org/r/74170/diff/1/?file=2270661#file2270661line207>
> >
> > 1. Any reason we are setting this as severe? Can we use the level WARN or ERROR?
> > 2. Can we pass the exception as parameter to the logger?
1. This class is using `java.util.logging.Logger` which does not have the level ERROR. It only has severe and warning
2. java.util.logging.Logger does not have method to pass exception as parameter in the `logger.warning` or `logger.severe` methods. It has `logger.log(Level, Exception, Supplier)` method but I didn't see it being used in this class so I also didn't use it
- Sai Sandeep
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74170/#review224798
-----------------------------------------------------------
On Oct. 14, 2022, 3:07 p.m., Sai Sandeep Rangisetti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74170/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2022, 3:07 p.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-3947
> https://issues.apache.org/jira/browse/RANGER-3947
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Closing the solr cloud client in SolrCollectionBootstrapper's retry loop of creating solr config and collection. Without this new solr cloud client is created in every loop and new connection pools which will not be cleaned up and create large number of threads
>
>
> Diffs
> -----
>
> embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java fe4006f76
>
>
> Diff: https://reviews.apache.org/r/74170/diff/1/
>
>
> Testing
> -------
>
> Ran ranger-admin without ranger_audit config in zk and no contrib/solr_for_audit_setup/conf file which leads to retry loop and verified that threads aren't increasinng
>
>
> Thanks,
>
> Sai Sandeep Rangisetti
>
>
Re: Review Request 74170: RANGER-3947 fix thread leak in SolrCollectionBootstrapper
Posted by Don Bosco Durai <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74170/#review224798
-----------------------------------------------------------
embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java
Lines 204 (patched)
<https://reviews.apache.org/r/74170/#comment313629>
Should set solrCloudClient to null after close? Also what happens if we set to null, would it get created again?
embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java
Lines 207 (patched)
<https://reviews.apache.org/r/74170/#comment313630>
1. Any reason we are setting this as severe? Can we use the level WARN or ERROR?
2. Can we pass the exception as parameter to the logger?
- Don Bosco Durai
On Oct. 14, 2022, 3:07 p.m., Sai Sandeep Rangisetti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74170/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2022, 3:07 p.m.)
>
>
> Review request for ranger.
>
>
> Bugs: RANGER-3947
> https://issues.apache.org/jira/browse/RANGER-3947
>
>
> Repository: ranger
>
>
> Description
> -------
>
> Closing the solr cloud client in SolrCollectionBootstrapper's retry loop of creating solr config and collection. Without this new solr cloud client is created in every loop and new connection pools which will not be cleaned up and create large number of threads
>
>
> Diffs
> -----
>
> embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/SolrCollectionBootstrapper.java fe4006f76
>
>
> Diff: https://reviews.apache.org/r/74170/diff/1/
>
>
> Testing
> -------
>
> Ran ranger-admin without ranger_audit config in zk and no contrib/solr_for_audit_setup/conf file which leads to retry loop and verified that threads aren't increasinng
>
>
> Thanks,
>
> Sai Sandeep Rangisetti
>
>