Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

On 26/10/18 16:14, Daniel Fuchs wrote: > Hi Rob, > > Looks better to me know. Though I admit that: > > 53 this.replies = new LinkedBlockingQueue<>(8 * > replyQueueCapacity / 10); > > is still a bit mystifying... Why not use the full > replyQueueCapacity provided? That doesn't look >

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hi Rob, Looks better to me know. Though I admit that: 53 this.replies = new LinkedBlockingQueue<>(8 * replyQueueCapacity / 10); is still a bit mystifying... Why not use the full replyQueueCapacity provided? That doesn't look strictly equivalent to the highWatermark logic that you

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Thanks again Daniel, http://cr.openjdk.java.net/~robm/8139965/webrev.04/ -Rob On 26/10/18 09:54, Daniel Fuchs wrote: > Hi Rob, > > Now I'm concerned about this method: > > 71 synchronized boolean addReplyBer(BerDecoder ber) { > 72 // check the closed boolean value here as

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hi Rob, Now I'm concerned about this method: 71 synchronized boolean addReplyBer(BerDecoder ber) { 72 // check the closed boolean value here as we don't want anything 73 // to be added to the queue after close() has been called. 74 if (cancelled || closed)

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Thanks Daniel: http://cr.openjdk.java.net/~robm/8139965/webrev.03/ I'm planning to follow up on the test side of things with a separate bug. I think the technique used in some of the recent SQE LDAP tests might be applicable. -Rob On 05/09/18 09:53, Daniel Fuchs wrote: > Hi Rob, > > That

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hi Rob, That looks better but I believe that: 1. closed should be volatile since it's read from outside synchronized block 2. it seems there might be a race where the last response received could be dropped, if the connection is closed just after it's been pulled from the queue. So I

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Thanks for the reviews folks. I believe the following captures your recommended changes: http://cr.openjdk.java.net/~robm/8139965/webrev.02/ W.r.t. testing I think this area has been difficult to test traditionally. I'll have a dig through the existing testbase (and I'll get back to you) to see

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hi Rob, I concur with Chris. completed needs to be volatile and close() needs to set a flag and use offer like cancel(). The condition for testing for closed then becomes that the flag is set and the queue is empty or has EOF as its head. Is there any way this could be tested by a regression

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Rob, > On 3 Sep 2018, at 22:48, Rob McKenna wrote: > > Hi folks, > > I'd like to get a re-review of this change: > > https://bugs.openjdk.java.net/browse/JDK-8139965 > This issue is closed as `will not fix`. I presume you will re-open it

Re: RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hi Rob, Code change looks good to me. Thanks, Vyom On Tuesday 04 September 2018 03:18 AM, Rob McKenna wrote: Hi folks, I'd like to get a re-review of this change: https://bugs.openjdk.java.net/browse/JDK-8139965 http://cr.openjdk.java.net/~robm/8139965/webrev/ In case the original has

RFR: 8139965 - Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hi folks, I'd like to get a re-review of this change: https://bugs.openjdk.java.net/browse/JDK-8139965 http://cr.openjdk.java.net/~robm/8139965/webrev/ In case the original has gone stale: http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-August/042767.html -Rob

Re: RFR : 8139965:Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

IIRC pausing the receiver was introduced to prevent an LDAP client (with limited memory) from being overwhelmed by server results and having no opportunity to transmit an abandon request. The 80% value was chosen to balance maximising queue capacity against the risk of an OOME. If that

Re: RFR : 8139965:Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hi Sean, I agree that the 80% reflects the previous coding and is ok. I don't have enough background on this code to know whether the 80% is now spurious and confusing and could be removed. Roger On 8/5/2016 11:46 AM, Seán Coffey wrote: Sorry - cc'ed the wrong Pavel earlier. Corrected. I

Re: RFR : 8139965:Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Sorry - cc'ed the wrong Pavel earlier. Corrected. I thought the consumer/producer model was best served by delegating to the BlockingQueue. Is there a need to synchronize as a result ? The 80% logic is only implemented in the rare case where the com.sun.jndi.ldap.search.replyQueueSize ldap

Re: RFR : 8139965:Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hi Sean, Looks like a cleaner solution to synchronize writer and readers. I don't quite understand the 80% capacity value. It is related to the obsolete highWatermark but that does not seem relevant with the update. If the caller is going to specify a replyQueueCapacity then why should it

RFR : 8139965:Hang seen when using com.sun.jndi.ldap.search.replyQueueSize

Hoping to get a review on this issue that's been sitting on my plate for a long while. Pavel drew up the bulk of the edits for this one (Thanks) The fix basically delegates polling and timeout management to the BlockingQueue.poll(timeout.. ) method. As a result it makes Connection readReply