OSDir


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [VOTE] Release 2.7.0, release candidate #1




Le mer. 12 sept. 2018 00:37, Lukasz Cwik <lcwik@xxxxxxxxxx> a écrit :
I was unaware that users would use multiple versions of Apache Beam on the classpath at the same time. In that case I don't believe shading is something that will be there number one problem since we don't have a stable API surface between internal Apache Beam components.

Agree was exactly what I tried to say.


For users who aren't using multiple Apache Beam packages, I would not expect non Apache Beam packages to ever export anything underneath the org.apache.beam package namespace.

Agree too.


Also, I did add tooling to our build process to make sure that we only release classes underneath the org.apache.beam package namespace with the validateShadedJarDoesntLeakNonOrgApacheBeamClasses[1] task.

Romain, I think this is something we could continue outside of the release thread. Feel free to start a new thread or follow up on Slack.

The point was that Beam is hiding non beam issues with such a delivery which is a blocker to upgrade. So beam alone is ok but if you add anything - and since you will likely for any pipeline - then your app is no more in a workable state while shades are a recommended solution.



On Tue, Sep 11, 2018 at 2:48 PM Romain Manni-Bucau <rmannibucau@xxxxxxxxx> wrote:
I understand Lukasz but it makes using shades properly pretty impossible since this warning is not just something you can ignore but something you have to fix since it can hide bugs. I get the "it is ok while you have a single beam version" point but why would you get only beam in your classpath, from the moment you use an IO it is not true anymore so this warning is key to ensure your deployment is under control. In general you accept something which fits the screen (like 20 overlapping classes or so) but having 6600 classes to check is way more than something which would be done just by a quick visual check. It requires you to add tooling on top of it which is not really good overall. Wonder if it wouldn't be better to revert that if it can't be completed short term and reapplied when possible (probably using a working branch).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mar. 11 sept. 2018 à 23:41, Lukasz Cwik <lcwik@xxxxxxxxxx> a écrit :
Romain, the beam-model-fn-execution-2.7.0.jar, beam-model-job-management-2.7.0.jar, beam-model-pipeline-2.7.0.jar have duplicates of the same classes to satisfy their dependencies (gRPC and protobuf and their transitive dependencies). Producing a separate artifact is still not done to prevent the message that your describing and other then size of jars, that message is benign in this case.

Note that much of our vendoring goal that the community had discussed and agreed upon is still not unfinished, for example Guava: https://issues.apache.org/jira/browse/BEAM-3608



On Tue, Sep 11, 2018 at 2:29 PM Romain Manni-Bucau <rmannibucau@xxxxxxxxx> wrote:
BTW, did you notice that doing a shade now logs something like:

[WARNING] beam-model-fn-execution-2.7.0.jar, beam-model-job-management-2.7.0.jar, beam-model-pipeline-2.7.0.jar define 6660 overlapping classes: 
[WARNING]   - org.apache.beam.vendor.netty.v4.io.netty.handler.codec.http.HttpClientCodec$1
[WARNING]   - org.apache.beam.vendor.guava.v20.com.google.common.util.concurrent.AggregateFutureState$SafeAtomicHelper
[WARNING]   - org.apache.beam.vendor.netty.v4.io.netty.util.concurrent.DefaultFutureListeners
[WARNING]   - org.apache.beam.vendor.netty.v4.io.netty.handler.ssl.OpenSslSessionContext$1
[WARNING]   - org.apache.beam.vendor.netty.v4.io.netty.handler.ssl.Java9SslUtils$4
[WARNING]   - org.apache.beam.vendor.guava.v20.com.google.common.collect.ImmutableMultimap$Builder
[WARNING]   - org.apache.beam.vendor.netty.v4.io.netty.handler.codec.spdy.SpdyHeaders
[WARNING]   - org.apache.beam.vendor.protobuf.v3.com.google.protobuf.DescriptorProtos$FieldDescriptorProtoOrBuilder
[WARNING]   - org.apache.beam.vendor.guava.v20.com.google.common.collect.AbstractMultimap
[WARNING]   - org.apache.beam.vendor.guava.v20.com.google.common.io.BaseEncoding$3
[WARNING]   - 6650 more...

Looks like the new shading policy impl was merged a bit too fast ;)

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mar. 11 sept. 2018 à 21:42, Jean-Baptiste Onofré <jb@xxxxxxxxxxxx> a écrit :
I'm taking the Spark runner one.

Regards
JB

On 11/09/2018 21:15, Ahmet Altay wrote:
> Could anyone else help with looking at these issues earlier?
>
> On Tue, Sep 11, 2018 at 12:03 PM, Romain Manni-Bucau
> <rmannibucau@xxxxxxxxx <mailto:rmannibucau@xxxxxxxxx>> wrote:
>
>     Im running this main [1] through this IT [2]. Was working fine since
>     ~1 year but 2.7.0 broke it. Didnt investigate more but can have a
>     look later this month if it helps.
>
>     [1]
>     https://github.com/Talend/component-runtime/blob/master/component-runtime-beam/src/it/serialization-over-cluster/src/main/java/org/talend/sdk/component/beam/it/clusterserialization/Main.java
>     <https://github.com/Talend/component-runtime/blob/master/component-runtime-beam/src/it/serialization-over-cluster/src/main/java/org/talend/sdk/component/beam/it/clusterserialization/Main.java>
>     [2]
>     https://github.com/Talend/component-runtime/blob/master/component-runtime-beam/src/it/serialization-over-cluster/src/test/java/org/talend/sdk/component/beam/it/SerializationOverClusterIT.java
>     <https://github.com/Talend/component-runtime/blob/master/component-runtime-beam/src/it/serialization-over-cluster/src/test/java/org/talend/sdk/component/beam/it/SerializationOverClusterIT.java>
>
>     Le mar. 11 sept. 2018 20:54, Charles Chen <ccy@xxxxxxxxxx
>     <mailto:ccy@xxxxxxxxxx>> a écrit :
>
>         Romain: can you give more details on the failure you're
>         encountering, i.e. how you are performing this validation?
>
>         On Tue, Sep 11, 2018 at 9:36 AM Jean-Baptiste Onofré
>         <jb@xxxxxxxxxxxx <mailto:jb@xxxxxxxxxxxx>> wrote:
>
>             Hi,
>
>             weird, I didn't have it on Beam samples. Let me try to
>             reproduce and I
>             will create the Jira.
>
>             Regards
>             JB
>
>             On 11/09/2018 11:44, Romain Manni-Bucau wrote:
>              > -1, seems spark integration is broken (tested with spark
>             2.3.1 and 2.2.1):
>              >
>              > 18/09/11 11:33:29 WARN TaskSetManager: Lost task 0.0 in
>             stage 0.0 (TID 0, RMANNIBUCAU, executor 0):
>             java.lang.ClassCastException: cannot assign instance of
>             scala.collection.immutable.List$SerializationProxy to
>             fieldorg.apache.spark.rdd.RDD.org
>             <http://fieldorg.apache.spark.rdd.RDD.org>
>             <http://org.apache.spark.rdd.RDD.org
>             <http://org.apache.spark.rdd.RDD.org>>$apache$spark$rdd$RDD$$dependencies_
>             of type scala.collection.Seq in instance of
>             org.apache.spark.rdd.MapPartitionsRDD
>              >       at
>             java.io.ObjectStreamClass$FieldReflector.setObjFieldValues(ObjectStreamClass.java:2233)
>              >
>              >
>              > Also the issue Lukasz identified is important even if
>             workarounds can be
>              > put in place so +1 to fix it as well if possible.
>              >
>              > Romain Manni-Bucau
>              > @rmannibucau <https://twitter.com/rmannibucau
>             <https://twitter.com/rmannibucau>> | Blog
>              > <https://rmannibucau.metawerx.net/
>             <https://rmannibucau.metawerx.net/>> | Old Blog
>              > <http://rmannibucau.wordpress.com
>             <http://rmannibucau.wordpress.com>> | Github
>              > <https://github.com/rmannibucau
>             <https://github.com/rmannibucau>> | LinkedIn
>              > <https://www.linkedin.com/in/rmannibucau
>             <https://www.linkedin.com/in/rmannibucau>> | Book
>              >
>             <https://www.packtpub.com/application-development/java-ee-8-high-performance
>             <https://www.packtpub.com/application-development/java-ee-8-high-performance>>
>              >
>              >
>              > Le lun. 10 sept. 2018 à 20:48, Lukasz Cwik
>             <lcwik@xxxxxxxxxx <mailto:lcwik@xxxxxxxxxx>
>              > <mailto:lcwik@xxxxxxxxxx <mailto:lcwik@xxxxxxxxxx>>> a
>             écrit :
>              >
>              >     I found an issue where we are no longer packaging the
>             pom.xml within
>              >     the artifact jars at
>             META-INF/maven/groupId/artifactId. More details
>              >     in https://issues.apache.org/jira/browse/BEAM-5351
>             <https://issues.apache.org/jira/browse/BEAM-5351>. I wouldn't
>              >     consider this a blocker but it was an easy fix
>              >     (https://github.com/apache/beam/pull/6358
>             <https://github.com/apache/beam/pull/6358>) and users may
>             rely on the
>              >     pom.xml.
>              >
>              >     Should we recut the release candidate to include this?
>              >
>              >     On Mon, Sep 10, 2018 at 4:58 AM Jean-Baptiste Onofré
>              >     <jb@xxxxxxxxxxxx <mailto:jb@xxxxxxxxxxxx>
>             <mailto:jb@xxxxxxxxxxxx <mailto:jb@xxxxxxxxxxxx>>> wrote:
>              >
>              >         +1 (binding)
>              >
>              >         Tested successfully on Beam Samples.
>              >
>              >         Thanks !
>              >
>              >         Regards
>              >         JB
>              >
>              >         On 07/09/2018 23:56, Charles Chen wrote:
>              >          > Hi everyone,
>              >          >
>              >          > Please review and vote on the release
>             candidate #1 for the
>              >         version
>              >          > 2.7.0, as follows:
>              >          > [ ] +1, Approve the release
>              >          > [ ] -1, Do not approve the release (please
>             provide specific
>              >         comments)
>              >          >
>              >          > The complete staging area is available for
>             your review, which
>              >         includes:
>              >          > * JIRA release notes [1],
>              >          > * the official Apache source release to be
>             deployed to
>              > dist.apache.org <http://dist.apache.org>
>             <http://dist.apache.org>
>              >          > <http://dist.apache.org> [2], which is signed
>             with the key with
>              >          > fingerprint 45C60AAAD115F560 [3],
>              >          > * all artifacts to be deployed to the Maven
>             Central
>              >         Repository [4],
>              >          > * source code tag "v2.7.0-RC1" [5],
>              >          > * website pull request listing the release and
>             publishing the API
>              >          > reference manual [6].
>              >          > * Java artifacts were built with Gradle 4.8
>             and OpenJDK
>              >          > 1.8.0_181-8u181-b13-1~deb9u1-b13.
>              >          > * Python artifacts are deployed along with the
>             source release
>              >         to the
>              >          > dist.apache.org <http://dist.apache.org>
>             <http://dist.apache.org>
>              >         <http://dist.apache.org> [2].
>              >          >
>              >          > The vote will be open for at least 72 hours.
>             It is adopted by
>              >         majority
>              >          > approval, with at least 3 PMC affirmative votes.
>              >          >
>              >          > Thanks,
>              >          > Charles
>              >          >
>              >          > [1]
>              >          >
>              >
>             https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12319527&version=12343654
>             <https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12319527&version=12343654>
>              >          > [2]
>             https://dist.apache.org/repos/dist/dev/beam/2.7.0
>             <https://dist.apache.org/repos/dist/dev/beam/2.7.0>
>              >          > [3]
>             https://dist.apache.org/repos/dist/dev/beam/KEYS
>             <https://dist.apache.org/repos/dist/dev/beam/KEYS>
>              >          > [4]
>              >
>             https://repository.apache.org/content/repositories/orgapachebeam-1046/
>             <https://repository.apache.org/content/repositories/orgapachebeam-1046/>
>              >          > [5]
>             https://github.com/apache/beam/tree/v2.7.0-RC1
>             <https://github.com/apache/beam/tree/v2.7.0-RC1>
>              >          > [6]
>             https://github.com/apache/beam-site/pull/549
>             <https://github.com/apache/beam-site/pull/549>
>              >
>              >         --
>              >         Jean-Baptiste Onofré
>              > jbonofre@xxxxxxxxxx <mailto:jbonofre@xxxxxxxxxx>
>             <mailto:jbonofre@xxxxxxxxxx <mailto:jbonofre@xxxxxxxxxx>>
>              > http://blog.nanthrax.net
>              >         Talend - http://www.talend.com
>              >
>
>