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
> strictly equivalent to the highWatermark logic that
> you have removed.
Pavel may be able to shed more light on this part. (as it was his
suggested fix originally I believe)
It looks to me like the intention is to mimic the pre-fix behaviour,
which only allowed the queue to fill to 80% of its capacity before
pausing. I agree though, it doesn't really make sense to keep that. I'll
remove it.
>
> On 25/10/2018 21:53, Rob McKenna wrote:
> > 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.
>
> It will be good to have a test and try to shake the implementation
> a bit with some repeating jobs in our test system to get some
> confidence that we've not harmed anything else.
You've convinced me to wait until we can get one together. I'll hold off
on the push until that point. jdk-tier1-4 pass, but that may not be
saying much unfortunately.
-Rob
>
> I admit that my only acquaintance to the JNDI/LDAP code of the JDK
> has been through reviews, so I'd probably only spot the obvious.
>
> best regards,
>
> -- daniel
>
>
> On 26/10/2018 15:55, Rob McKenna wrote:
> > Thanks again Daniel,
> >
> > http://cr.openjdk.java.net/~robm/8139965/webrev.04/
> >
> > -Rob
> >
>
