Matteo, https://github.com/apache/bookkeeper/pull/2649 also a good candidate for this release (fits the compaction improvements theme), also has changes requested
On Fri, May 7, 2021 at 3:01 PM Andrey Yegorov <andrey.yego...@datastax.com> wrote: > 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 > -- -- Andrey Yegorov
