You are viewing a plain text version of this content. The canonical link for it is here.

Posted to solr-user@lucene.apache.org by "Trym R. Møller" <tr...@sigmat.dk> on 2012/11/09 14:41:34 UTC

SolrZKClient changed interface

Hi

The constructor of SolrZKClient has changed, I expect to ensure clean up 
of resources. The strategy is as follows:
connManager = new ConnectionManager(...)
try {
     ...
} catch (Throwable e) {
       connManager.close();
       throw new RuntimeException();
}
try {
   connManager.waitForConnected(clientConnectTimeout);
} catch (Throwable e) {
   connManager.close();
   throw new RuntimeException();
}

This results in a different exception (RuntimeException) returned from 
the constructor as earlier (nice exceptions as UnknownHostException, 
TimeoutException).

Can this be changed so we keep the old nice exceptions e.g. as follows 
(requiring the constructor to declare these) or at least include them as 
cause in the RuntimeException?

boolean closeBecauseOfException = true;
try {
     ...
    connManager.waitForConnected(clientConnectTimeout);
    closeBecauseOfException = false
} finally {
     if (closeBecauseOfException) {
         connManager.close();
     }
}

Any comments appreciated.

Best regards Trym

http://svn.apache.org/repos/asf/lucene/dev/branches/lucene_solr_4_0/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java

Re: SolrZKClient changed interface

Posted by Per Steffensen <st...@designware.dk>.

Hi Trym

I believe one of the reasons that they started throwing 
RuntimeExceptions insted of UnknownHostException, TimeoutException etc 
is that the method signature has changed to not have a "throws"-part. 
They probably do not want do deal with those checked exceptions. Im not 
sure I completely understand your suggestion, but if they want to keep 
the method signature this way, there is not way to get checked 
excpetions (like UnknownHostException and TimeoutException) out of this 
method - therefore best solution is at least to include the actual 
exceptions as cause on the RuntimeExceptions. Or maybe it is just an 
accident that the checked excpetions was removed from the method 
signature, and then of course you can add them again at make sure the 
actual exceptions are propagated correctly.

Regards, Per Steffensen

Trym R. Møller skrev:
> Hi
>
> The constructor of SolrZKClient has changed, I expect to ensure clean 
> up of resources. The strategy is as follows:
> connManager = new ConnectionManager(...)
> try {
>     ...
> } catch (Throwable e) {
>       connManager.close();
>       throw new RuntimeException();
> }
> try {
>   connManager.waitForConnected(clientConnectTimeout);
> } catch (Throwable e) {
>   connManager.close();
>   throw new RuntimeException();
> }
>
> This results in a different exception (RuntimeException) returned from 
> the constructor as earlier (nice exceptions as UnknownHostException, 
> TimeoutException).
>
> Can this be changed so we keep the old nice exceptions e.g. as follows 
> (requiring the constructor to declare these) or at least include them 
> as cause in the RuntimeException?
>
> boolean closeBecauseOfException = true;
> try {
>     ...
>    connManager.waitForConnected(clientConnectTimeout);
>    closeBecauseOfException = false
> } finally {
>     if (closeBecauseOfException) {
>         connManager.close();
>     }
> }
>
> Any comments appreciated.
>
> Best regards Trym
>
> http://svn.apache.org/repos/asf/lucene/dev/branches/lucene_solr_4_0/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java 
>
>

Re: SolrZKClient changed interface

Posted by "Trym R. Møller" <tr...@sigmat.dk>.

Can anyone verify that the jira has been created sensible?
Thanks in advance.

https://issues.apache.org/jira/browse/SOLR-4066

Best regards Trym

Den 10-11-2012 00:54, Mark Miller skrev:
> Please file a JIRA issue for this change.
>
> - Mark
>
> On Nov 9, 2012, at 8:41 AM, Trym R. Møller <tr...@sigmat.dk> wrote:
>
>> Hi
>>
>> The constructor of SolrZKClient has changed, I expect to ensure clean up of resources. The strategy is as follows:
>> connManager = new ConnectionManager(...)
>> try {
>>     ...
>> } catch (Throwable e) {
>>       connManager.close();
>>       throw new RuntimeException();
>> }
>> try {
>>   connManager.waitForConnected(clientConnectTimeout);
>> } catch (Throwable e) {
>>   connManager.close();
>>   throw new RuntimeException();
>> }
>>
>> This results in a different exception (RuntimeException) returned from the constructor as earlier (nice exceptions as UnknownHostException, TimeoutException).
>>
>> Can this be changed so we keep the old nice exceptions e.g. as follows (requiring the constructor to declare these) or at least include them as cause in the RuntimeException?
>>
>> boolean closeBecauseOfException = true;
>> try {
>>     ...
>>    connManager.waitForConnected(clientConnectTimeout);
>>    closeBecauseOfException = false
>> } finally {
>>     if (closeBecauseOfException) {
>>         connManager.close();
>>     }
>> }
>>
>> Any comments appreciated.
>>
>> Best regards Trym
>>
>> http://svn.apache.org/repos/asf/lucene/dev/branches/lucene_solr_4_0/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java

Re: SolrZKClient changed interface

Posted by Mark Miller <ma...@gmail.com>.

Please file a JIRA issue for this change.

- Mark

On Nov 9, 2012, at 8:41 AM, Trym R. Møller <tr...@sigmat.dk> wrote:

> Hi
> 
> The constructor of SolrZKClient has changed, I expect to ensure clean up of resources. The strategy is as follows:
> connManager = new ConnectionManager(...)
> try {
>    ...
> } catch (Throwable e) {
>      connManager.close();
>      throw new RuntimeException();
> }
> try {
>  connManager.waitForConnected(clientConnectTimeout);
> } catch (Throwable e) {
>  connManager.close();
>  throw new RuntimeException();
> }
> 
> This results in a different exception (RuntimeException) returned from the constructor as earlier (nice exceptions as UnknownHostException, TimeoutException).
> 
> Can this be changed so we keep the old nice exceptions e.g. as follows (requiring the constructor to declare these) or at least include them as cause in the RuntimeException?
> 
> boolean closeBecauseOfException = true;
> try {
>    ...
>   connManager.waitForConnected(clientConnectTimeout);
>   closeBecauseOfException = false
> } finally {
>    if (closeBecauseOfException) {
>        connManager.close();
>    }
> }
> 
> Any comments appreciated.
> 
> Best regards Trym
> 
> http://svn.apache.org/repos/asf/lucene/dev/branches/lucene_solr_4_0/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java