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 behaviour is maintained via a BlockingQueue then all is well. > On 5 Aug 2016, at 16:56, Roger Riggs <roger.ri...@oracle.com> wrote: > > 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 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 context property is set. It >> seems to be legacy behaviour from the old code where the reader thread was >> paused at 80% capacity. Setting the BlockingQueue to the same '80%' size >> should have the same net effect. >> >> Regards, >> Sean. >> >> On 05/08/16 15:50, Roger Riggs wrote: >>> 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 >>> be downgraded to 80%? >>> >>> Roger >>> >>> >>> >>> On 8/5/2016 10:05 AM, Seán Coffey wrote: >>>> 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 logic much easier to handle. >>>> >>>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8139965.9/webrev/ >>>> bug report : https://bugs.openjdk.java.net/browse/JDK-8139965 >>>> >>> >> >
