Re: Severe performance degradation when using the 9.2-1000 JDBC 4 driver

On Fri, 19 Oct 2012, Scott Harrington wrote:

> On Fri, 19 Oct 2012, Craig Ringer wrote:
>
>> On 10/19/2012 02:36 PM, Mikko Tiihonen wrote:
>>>>> I think the DNS lookup is part of the JDBC failover connection patch
>>>>> that I created. I initially used a InetSocketAddress.getHostString()
>>>>> that does not do any DNS lookups, but since that method was added in
>>>>> Java7 I had to revert to getHostName() method which does.
>>>>> 
>>>> 
>>>> Is there any functionality that we need that is provided by using
>>>> InetSocketAddress or is it just a convenient object to pass around host
>>>> and port together?  The simplest option to me seems to be reverting to
>>>> passing simple strings/ints around (possibly in some kind of our own
>>>> container).
>>> 
>>> It is just a convenient container. And it seems on older JVMs it cannot be 
>>> used for such purposes.
>>> I agree that we should just replace it with our own container that 
>>> contains the host/port pair.
>> 
>> If that's all you're using it for, then that's the simplest and best 
>> choice. +1 from me on that.
>
> Could the new container contain the localAddress (null for "any") as well as 
> the target host and port? I'm using a simple patch that allows binding my 
> JDBC connections to specific localAddress (unit tests & docs still to-do), 
> and I can envision scenarios where a client with multiple NICs might need to 
> initiate connections to failover hostA from localAddress X and to failover 
> hostB from localAddress Y, in environments where iptables SNAT may not be an 
> option.
>
> By creating an appropriate factory class for this laddr/caddr/cport container 
> we could select the best implementation (Java7-only or pre-Java7) in a static 
> initializer and avoid having to resort to reflection to avoid the unwanted 
> DNS calls as discussed earlier in this thread. Want me to take a stab at this 
> this weekend? I'm not git-skilled yet so I'll just have to email a patch.

Hi everyone, please look over this patch.

By introducing a simple org.postgresql.util.HostSpec to contain the 
orignal caller-provided host and port, we never even need to call 
InetSocketAddress.getHostName() or getHostString().

If this is accepted, then I will resubmit the localAddress patch based on 
this; will be quite simple to put localAddress in HostSpec and then it's 
available where needed at the time of Socket.connect in PGStream without 
having to touch a dozen other constructor signatures to get it there.