ok, I'll hold on on the release until these two are ready to merge Both PRs have changes requested by Enrico. I'll postpone my attempt to build the release until Monday.
If there are other PRs that *have* to be in 4.14 please move them back to the 4.14 milestone and update this thread. On Fri, May 7, 2021 at 1:06 PM Matteo Merli <mme...@apache.org> wrote: > Andrey, there are several PRs that would be good to get into 4.14. We > shouldn't just push everything out to 4.15. > > Just a couple of examples: > * https://github.com/apache/bookkeeper/pull/2710 > * https://github.com/apache/bookkeeper/pull/2698 > > > > -- > Matteo Merli > <mme...@apache.org> > > On Fri, May 7, 2021 at 12:55 PM Andrey Yegorov > <andrey.yego...@datastax.com> wrote: > > > > I created https://github.com/apache/bookkeeper/pull/2712 with docs and > > release notes update for the v 4.14.0 > > The most interesting part is the release notes: > > > https://github.com/apache/bookkeeper/blob/e3c5994c05c970e6343fa9b43d1e63bac6142e60/site/docs/4.14.0/overview/releaseNotes.md > > > > Some PRs missed milestones and/or release labels, probably merged > manually. > > I tracked changelists from git history and updated the labels/milestones. > > > > I'll start working on the release. > > > > On Fri, May 7, 2021 at 8:52 AM Andrey Yegorov < > andrey.yego...@datastax.com> > > wrote: > > > > > I added https://github.com/apache/bookkeeper/issues/2711 for the TLS > 1.3 > > > support > > > Unless someone objects in the next 30min, I'll merge Lari's PR. > > > After that I'll start working on the BK 4.14.0 release. > > > > > > On Thu, May 6, 2021 at 1:25 AM Lari Hotari <l...@hotari.net> wrote: > > > > > >> I can confirm that the PR checks pass after excluding TLSv1.3 from > enabled > > >> protocols: > > >> > > >> > https://github.com/apache/bookkeeper/pull/2696/commits/6003a374d5aec30d7059a21e473ac91417b5cdc3 > > >> > > >> There should be tests for both TLSv1.2 and TLSv1.3 because of the > > >> differences in TLS handshake described in > > >> > > >> > https://urldefense.proofpoint.com/v2/url?u=https-3A__stackoverflow.com_a_62465859&d=DwIFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=0B1UvYMwy7dr9qtqFwQCfxUyrozUgZzbOshynTIaYUY&m=76JE79AuinlMNecD5DDFGgg-jXzCGZEh3PANpQOJUoE&s=iZz_eExfeElZI--ooxMmyMABWjailhDc7rKIAZNg59s&e= > > >> . > > >> > > >> This also impacts some production code in Bookkeeper. The PR already > > >> includes a change to catch SSLException instead of > SSLHandshakeException ( > > >> > > >> > https://github.com/apache/bookkeeper/pull/2696/commits/fcbd707a633ed1b8cf8290cb5d70a3070e010196 > > >> ). > > >> TLSv1.3 doesn't throw SSLHandshakeException for certificate issues > because > > >> of the differences in the protocols. This change should work for both > > >> protocols, but we should have test coverage to ensure that. > > >> > > >> TLSv1.3 has been enabled by default since Netty 4.1.52.Final (when > the JDK > > >> contains TLSv1.3). TLSv1.3 support has been available in Java 8 since > > >> 8u262 > > >> . > > >> > > >> One of the remaining problems with TLSv1.3 support in BK is the state > > >> machine and TLS counters in PerChannelBookieClient . It doesn't > properly > > >> model the way TLS 1.3 behaves. Currently there's a counter > > >> FAILED_TLS_HANDSHAKE_COUNTER which is expected to count also the > > >> certificate issues (code: > > >> > > >> > https://github.com/apache/bookkeeper/blob/fcbd707a633ed1b8cf8290cb5d70a3070e010196/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java#L1535-L1543 > > >> ). Since TLSv1.3 doesn't detect certificate issues (mutual TLS) during > > >> handshake, this counter doesn't count certificate issues. Certificate > > >> issues will show up as successfully established connections. > > >> The original issue for adding TLS counters was > > >> https://github.com/apache/bookkeeper/issues/1103 and PR commit was > > >> > > >> > https://github.com/apache/bookkeeper/commit/fa10b7dcd89c40222ba5f30bb60f785bd21669b2 > > >> . > > >> > > >> How do we revisit the TLS counter solution for TLSv1.3 compatibility? > Do > > >> we > > >> make changes to the code or do we simply skip the test on TLSv1.3 > which > > >> ensures that a certificate issue is counted in > > >> FAILED_TLS_HANDSHAKE_COUNTER > > >> ? Skipping the test for TLSv1.3 would be one option. WDYT? > > >> > > >> -Lari > > >> > > >> On Wed, May 5, 2021 at 10:32 PM Andrey Yegorov < > > >> andrey.yego...@datastax.com> > > >> wrote: > > >> > > >> > Lari and I have looked at the Netty upgrade. > > >> > There are some test breaks, and so far everything is related to > behavior > > >> > changes related to TLS 1.3, see > > >> > https://urldefense.proofpoint.com/v2/url?u=https-3A__stackoverflow.com_a_62465859&d=DwIFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=0B1UvYMwy7dr9qtqFwQCfxUyrozUgZzbOshynTIaYUY&m=76JE79AuinlMNecD5DDFGgg-jXzCGZEh3PANpQOJUoE&s=iZz_eExfeElZI--ooxMmyMABWjailhDc7rKIAZNg59s&e= > > >> > We managed to fix some of the issues > > >> > https://github.com/apache/bookkeeper/pull/2696 but "the client > won't > > >> know > > >> > whether the server has accepted the certificate or not until it next > > >> reads > > >> > data from the server" complicates things. > > >> > > > >> > Currently we are considering simply setting "java > > >> > -Djdk.tls.client.protocols=TLSv1.2" to unbreak the tests and > handling > > >> > tls1.3 as a separate work item. > > >> > > > >> > Lari is planning on spending a little bit more time on this tomorrow > > >> (his > > >> > tomorrow) to see if there is a better way to address this quickly; > we'll > > >> > hear more then. > > >> > > > >> > > > >> > On Wed, May 5, 2021 at 9:23 AM Henry Saputra < > henry.sapu...@gmail.com> > > >> > wrote: > > >> > > > >> > > I am +1 for having next release as 4.14.0 > > >> > > > > >> > > - Henry > > >> > > > > >> > > On Tue, May 4, 2021 at 2:51 PM Andrey Yegorov < > > >> > andrey.yego...@datastax.com > > >> > > > > > >> > > wrote: > > >> > > > > >> > > > Overall +1 for 4.14.0 - the milestone is due May 16th anyway. > > >> > > > There is nothing that breaks compatibility with 4.13 so we can > skip > > >> > > 4.13.1. > > >> > > > > > >> > > > One thing I'd love to see in 4.14 is > > >> > > > https://github.com/apache/bookkeeper/pull/2696 , to fix > > >> > > > https://github.com/netty/netty/issues/10986 > > >> > > > It looks like there are issues with vertx > > >> > > > > > >> https://github.com/apache/bookkeeper/pull/2693#issuecomment-823774769 > > >> > > > I hope we can upgrade to latest vertx 3.9.7 and netty 4.1.60 > > >> > > > > > >> > > > Also https://github.com/apache/bookkeeper/pull/2695 upgrades > > >> libthrift > > >> > > to > > >> > > > address security issues. > > >> > > > > > >> > > > All these PRs are from Lari, I'll follow up with him. > > >> > > > > > >> > > > I can be a RM if there are no other volunteers. > > >> > > > > > >> > > > > > >> > > > On Tue, May 4, 2021 at 1:57 PM Matteo Merli <mme...@apache.org> > > >> wrote: > > >> > > > > > >> > > > > +1 We should do 4.14, carrying all the fixes that are there in > > >> master > > >> > > as > > >> > > > > well. > > >> > > > > > > >> > > > > > > >> > > > > -- > > >> > > > > Matteo Merli > > >> > > > > <mme...@apache.org> > > >> > > > > > > >> > > > > On Tue, May 4, 2021 at 11:57 AM Sijie Guo <guosi...@gmail.com > > > > >> > wrote: > > >> > > > > > > > >> > > > > > +1 > > >> > > > > > > > >> > > > > > On Tue, May 4, 2021 at 2:22 AM Yunze Xu > > >> > <y...@streamnative.io.invalid > > >> > > > > > >> > > > > > wrote: > > >> > > > > > > > >> > > > > > > Hello, > > >> > > > > > > About 10 days ago I found a heap memory copy problem in > Apache > > >> > > > Pulsar, > > >> > > > > see > > >> > > > > > > [1]. > > >> > > > > > > It’s a problem of BK side because when > > >> > `LedgerHandle#asyncAddEntry` > > >> > > > > > > accepts a `CompositeByteBuf` or a wrapper, it will finally > > >> call > > >> > > > > > > `ByteBuf#nioBuffer()`, which would make a heap copy from > > >> direct > > >> > > > memory. > > >> > > > > > > [2] fixed this problem and has been merged for a week. > > >> > > > > > > > > >> > > > > > > Since it has a significant impact on Pulsar, Pulsar side > > >> needs a > > >> > > new > > >> > > > BK > > >> > > > > > > release with [2] merged to fix it. Is there any plan to > cut a > > >> > > 4.13.1 > > >> > > > > > > release or 4.14.0 release so that we can upgrade the > > >> dependency > > >> > in > > >> > > > > Pulsar? > > >> > > > > > > > > >> > > > > > > Thanks, > > >> > > > > > > Yunze > > >> > > > > > > > > >> > > > > > > [1] https://github.com/apache/pulsar/pull/10330 < > > >> > > > > > > https://github.com/apache/pulsar/pull/10330> > > >> > > > > > > > > >> > > > > > > [2] https://github.com/apache/bookkeeper/pull/2701 < > > >> > > > > > > https://github.com/apache/bookkeeper/pull/2701> > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > >> > > > > > >> > > > > > >> > > > -- > > >> > > > > > >> > > > -- > > >> > > > Andrey Yegorov > > >> > > > > > >> > > > > >> > > > >> > > > >> > -- > > >> > > > >> > -- > > >> > Andrey Yegorov > > >> > > > >> > > > > > > > > > -- > > > > > > -- > > > Andrey Yegorov > > > > > > > > > -- > > > > -- > > Andrey Yegorov > -- -- Andrey Yegorov
