+1 on cancelling the vote and flowing thru with new vote one these issues are 
fixed.
From ozone perspective, we can take the Ratis bits and run a full cluster level 
test and provide feedback on the next RC.

Thanks
Anu


On 5/8/18, 2:16 PM, "Elek, Marton" <e...@apache.org> wrote:

    
    Hi,
    
    thank you the test and feedback for all of you.
    
    As a summary. The problems until now:
    
      1. Unit test failures
      2. OpenConsensus shading problem: RATIS-237
      3. Copyright 2017
      4. Missing information about increasing MVN memory.
      5. Licence headers in shaded jar files
    
    
    
    3/4: are not blockers (IMHO)
    
    2: I think it's a blocker. As some of our unit tests are fragile and 
    intermittent (1.) Hadoop Ozone could give us additional confidence about 
    the stability of the release. Also we would like to provide a stable 
    Ratis version to Ozone, so they should work together.
    
    I would cancel this vote. And initialize a new one with RATIS-237 (+ the 
    trival fixes).
    
    5. I am not sure if it's blocker or not (as LICENSE.txt is ok), but I 
    think the shaded sources should not be included in the source 
    distribution. I think they are  just external dependencies similar to 
    other maven dependencies (just on the source level). I would fix it with 
    removing the src/main/java of the shaded project from the src release.
    
    1. As I mentioned some of the unit tests seem to be fragile (see 
    https://builds.apache.org/job/ratis-qbt-master-java8-linux-x86/). I 
    don't think it's a blocker (but would be great to fix them long-term).
    
    Summary: I cancel this vote and propose to include the fix of 2,3,4,5 in 
    rc1 (hopefully very soon).
    
    But this is just my view. Please let me know what do you think...
    
    Thanks a lot,
    Marton
    
    
    
    
    
    
    
    
    On 05/08/2018 10:08 PM, Tsz Wo Sze wrote:
    > Thanks Nanda for the explanation.
    > 
    > I seem able to fix the io.opencensus shading problem.  Just have
    > posted a patch on RATIS-237.
    > 
    > Tsz-Wo
    > 
    > On Mon, May 7, 2018 at 8:28 PM, Nandakumar Vadivelu
    > <nvadiv...@hortonworks.com> wrote:
    >> Hi Tsz-Wo,
    >>
    >> True, there is only one version of opencensus in Ratis. Since we do 
shading of io.grpc we end up with new shaded class of ContextUtils (same fully 
qualified class name, but the reference of io.grpc inside the class is 
modified/relocated), we also have non shaded ContextUtils. Both are referred 
through Ratis dependency.
    >>
    >> The below is dependency tree of Ratis:
    >>
    >> [INFO] +- 
org.apache.ratis:ratis-proto-shaded:jar:0.1.1-alpha-SNAPSHOT:compile            
=========> Modified opencensus packed with proto-shaded
    >> [INFO] |  +- com.google.auto.value:auto-value-annotations:jar:1.6:compile
    >> [INFO] |  +- com.google.guava:guava:jar:24.1-jre:compile
    >> [INFO] |  |  +- com.google.code.findbugs:jsr305:jar:1.3.9:compile
    >> [INFO] |  |  +- 
org.checkerframework:checker-compat-qual:jar:2.0.0:compile
    >> [INFO] |  |  +- 
com.google.errorprone:error_prone_annotations:jar:2.1.3:compile
    >> [INFO] |  |  +- com.google.j2objc:j2objc-annotations:jar:1.1:compile
    >> [INFO] |  |  \- 
org.codehaus.mojo:animal-sniffer-annotations:jar:1.14:compile
    >> [INFO] |  \- com.squareup:javapoet:jar:1.10.0:compile
    >> [INFO] +- org.apache.ratis:ratis-common:jar:0.1.1-alpha-SNAPSHOT:compile
    >> [INFO] |  +- org.slf4j:slf4j-api:jar:1.7.10:compile
    >> [INFO] |  +- org.slf4j:slf4j-log4j12:jar:1.7.10:compile
    >> [INFO] |  |  \- log4j:log4j:jar:1.2.17:compile
    >> [INFO] |  \- io.dropwizard.metrics:metrics-core:jar:3.2.5:compile
    >> [INFO] +- org.apache.ratis:ratis-client:jar:0.1.1-alpha-SNAPSHOT:compile
    >> [INFO] +- org.apache.ratis:ratis-server:jar:0.1.1-alpha-SNAPSHOT:compile
    >> [INFO] +- org.apache.ratis:ratis-netty:jar:0.1.1-alpha-SNAPSHOT:compile
    >> [INFO] |  \- org.jctools:jctools-core:jar:2.1.2:compile
    >> [INFO] \- org.apache.ratis:ratis-grpc:jar:0.1.1-alpha-SNAPSHOT:compile
    >> [INFO]    +- io.opencensus:opencensus-api:jar:0.12.2:compile             
               =========> Original jar
    >> [INFO]    |  \- io.grpc:grpc-context:jar:1.9.0:compile
    >> [INFO]    \- 
io.opencensus:opencensus-contrib-grpc-metrics:jar:0.12.2:compile
    >>
    >>
    >> nvadivelu@HW12726 ~/w/t/p/o/t/dependency> javap -classpath 
".:./opencensus-api-0.12.2.jar" io.opencensus.trace.unsafe.ContextUtils
    >> Compiled from "ContextUtils.java"
    >> public final class io.opencensus.trace.unsafe.ContextUtils {
    >>    public static final io.grpc.Context$Key<io.opencensus.trace.Span> 
CONTEXT_SPAN_KEY;
    >>    static {};
    >> }
    >>
    >> nvadivelu@HW12726 ~/w/t/p/o/t/dependency> javap -classpath 
".:./ratis-proto-shaded-0.1.1-alpha-SNAPSHOT.jar" 
io.opencensus.trace.unsafe.ContextUtils
    >> Compiled from "ContextUtils.java"
    >> public final class io.opencensus.trace.unsafe.ContextUtils {
    >>    public static final 
org.apache.ratis.shaded.io.grpc.Context$Key<io.opencensus.trace.Span> 
CONTEXT_SPAN_KEY;
    >>    static {};
    >> }
    >>
    >>
    >> -Nanda
    >>
    >> On 5/8/18, 4:16 AM, "Tsz Wo Sze" <szets...@gmail.com> wrote:
    >>
    >>      > The problem is because of two different definition of 
io.opencensus.trace.unsafe.ContextUtils class present in the classpath. The 
reason for having two different definition of ContextUtils in classpath is due 
to the fact that we are shading/relocating io.grpc and not io.opencensus.
    >>
    >>      Nanda, in Ratis, there is only one version (0.12.2) of opencensus.  
It
    >>      seems that another version of opencensus is pulled outside Ratis.
    >>
    >>      Tsz-Wo
    >>
    >>
    >>      On Mon, May 7, 2018 at 11:58 AM, Nandakumar Vadivelu
    >>      <nvadiv...@hortonworks.com> wrote:
    >>      > I'm not sure why the file is missing, I can see the attachment in 
my "sent items".
    >>      >
    >>      > Patch content for reference
    >>      >
    >>      > diff --git a/ratis-grpc/pom.xml b/ratis-grpc/pom.xml
    >>      > index a3c3cc1..ebb73cc 100644
    >>      > --- a/ratis-grpc/pom.xml
    >>      > +++ b/ratis-grpc/pom.xml
    >>      > @@ -85,15 +85,5 @@
    >>      >        <artifactId>jctools-core</artifactId>
    >>      >      </dependency>
    >>      >
    >>      > -    <dependency>
    >>      > -      <groupId>io.opencensus</groupId>
    >>      > -      <artifactId>opencensus-api</artifactId>
    >>      > -      <version>${io.opencensus.version}</version>
    >>      > -    </dependency>
    >>      > -    <dependency>
    >>      > -      <groupId>io.opencensus</groupId>
    >>      > -      <artifactId>opencensus-contrib-grpc-metrics</artifactId>
    >>      > -      <version>${io.opencensus.version}</version>
    >>      > -    </dependency>
    >>      >    </dependencies>
    >>      >  </project>
    >>      >
    >>      > I will also create a jira under RATIS
    >>      >
    >>      > -Nanda
    >>      >
    >>      > On 5/8/18, 12:23 AM, "Ted Yu" <yuzhih...@gmail.com> wrote:
    >>      >
    >>      >     I don't see patch attached.
    >>      >
    >>      >     Please create RATIS JIRA and attach there.
    >>      >
    >>      >     Thanks
    >>      >
    >>      >     On Mon, May 7, 2018 at 10:24 AM, Nandakumar Vadivelu <
    >>      >     nvadiv...@hortonworks.com> wrote:
    >>      >
    >>      >     > Hi All,
    >>      >     >
    >>      >     > Below is the exception which we get with Ratis 0.2.0
    >>      >     >
    >>      >     > java.lang.NoSuchFieldError: CONTEXT_SPAN_KEY
    >>      >     >   at 
org.apache.ratis.shaded.io.grpc.internal.CensusTracingModule$
    >>      >     > ServerTracer.filterContext(CensusTracingModule.java:340)
    >>      >     >   at 
org.apache.ratis.shaded.io.grpc.internal.StatsTraceContext.
    >>      >     > serverFilterContext(StatsTraceContext.java:121)
    >>      >     >   at org.apache.ratis.shaded.io.grpc.internal.ServerImpl$
    >>      >     > 
ServerTransportListenerImpl.createContext(ServerImpl.java:482)
    >>      >     >   at org.apache.ratis.shaded.io.grpc.internal.ServerImpl$
    >>      >     > 
ServerTransportListenerImpl.streamCreated(ServerImpl.java:416)
    >>      >     > <stack trace truncated>
    >>      >     >
    >>      >     >
    >>      >     > The problem is because of two different definition of
    >>      >     > io.opencensus.trace.unsafe.ContextUtils class present in 
the classpath.
    >>      >     > The reason for having two different definition of 
ContextUtils in classpath
    >>      >     > is due to the fact that we are shading/relocating io.grpc 
and not
    >>      >     > io.opencensus.
    >>      >     >
    >>      >     >
    >>      >     > With shading we are generating the below definition which 
ends up in
    >>      >     > ratis-proto-shaded jar
    >>      >     >
    >>      >     > package io.opencensus.trace.unsafe;
    >>      >     >
    >>      >     > import org.apache.ratis.shaded.io.grpc.Context;
    >>      >     > import io.opencensus.trace.Span;
    >>      >     >
    >>      >     > /**
    >>      >     >  * Util methods/functionality to interact with the {@link
    >>      >     > org.apache.ratis.shaded.io.grpc.Context}.
    >>      >     >  *
    >>      >     >  * <p>Users must interact with the current Context via the 
public APIs in
    >>      >     > {@link
    >>      >     >  * io.opencensus.trace.Tracer} and avoid usages of the 
{@link
    >>      >     > #CONTEXT_SPAN_KEY} directly.
    >>      >     >  *
    >>      >     >  * @since 0.5
    >>      >     >  */
    >>      >     > public final class ContextUtils {
    >>      >     >   // No instance of this class.
    >>      >     >   private ContextUtils() {}
    >>      >     >
    >>      >     >   /**
    >>      >     >    * The {@link 
org.apache.ratis.shaded.io.grpc.Context.Key} used to
    >>      >     > interact with {@link 
org.apache.ratis.shaded.io.grpc.Context}.
    >>      >     >    *
    >>      >     >    * @since 0.5
    >>      >     >    */
    >>      >     >   public static final Context.Key<Span> CONTEXT_SPAN_KEY =
    >>      >     > Context.key("opencensus-trace-span-key");
    >>      >     > }
    >>      >     >
    >>      >     >
    >>      >     > Since we have added io.opencensus as direct dependency in 
ratis-grpc we
    >>      >     > get the below definition in io.opencensus:opencensus-api:jar
    >>      >     >
    >>      >     > package io.opencensus.trace.unsafe;
    >>      >     >
    >>      >     > import io.grpc.Context;
    >>      >     > import io.opencensus.trace.Span;
    >>      >     >
    >>      >     > /**
    >>      >     >  * Util methods/functionality to interact with the {@link 
io.grpc.Context}.
    >>      >     >  *
    >>      >     >  * <p>Users must interact with the current Context via the 
public APIs in
    >>      >     > {@link
    >>      >     >  * io.opencensus.trace.Tracer} and avoid usages of the 
{@link
    >>      >     > #CONTEXT_SPAN_KEY} directly.
    >>      >     >  *
    >>      >     >  * @since 0.5
    >>      >     >  */
    >>      >     > public final class ContextUtils {
    >>      >     >   // No instance of this class.
    >>      >     >   private ContextUtils() {}
    >>      >     >
    >>      >     >   /**
    >>      >     >    * The {@link io.grpc.Context.Key} used to interact with 
{@link
    >>      >     > io.grpc.Context}.
    >>      >     >    *
    >>      >     >    * @since 0.5
    >>      >     >    */
    >>      >     >   public static final Context.Key<Span> CONTEXT_SPAN_KEY =
    >>      >     > Context.key("opencensus-trace-span-key");
    >>      >     > }
    >>      >     >
    >>      >     > Both of the jars end up in classpath and ContextUtils is 
getting loaded
    >>      >     > from io.opencensus:opencensus-api:jar which is causing the 
problem.
    >>      >     >
    >>      >     > The simple fix would be to remove the dependency of 
io.opencensus from
    >>      >     > ratis-grpc/pom.xml (patch for the same is attached in this 
mail). This is
    >>      >     > not a permanent fix, as any application might bring in
    >>      >     > io.opencensus:opencensus-api:jar of its own.
    >>      >     >
    >>      >     > We have to find a way to shade 
io.opencensus:opencensus-api:jar.
    >>      >     >
    >>      >     > Note: from the comment in ratis-proto-shaded/pom.xml: 
Cannot relocate
    >>      >     > io.opencensus due to AutoValue code generation
    >>      >     >
    >>      >     >
    >>      >     > -Nanda
    >>      >     >
    >>      >     > On 5/7/18, 3:28 PM, "Lokesh Jain" <lj...@hortonworks.com> 
wrote:
    >>      >     >
    >>      >     >     We found an issue while updating ozone to Ratis 
snapshot build
    >>      >     > 0.1.1-alpha-4309324-SNAPSHOT (https://issues.apache.org/
    >>      >     > jira/browse/HDDS-19). I feel we should hold the release 
till this is
    >>      >     > fixed. Please let us know of your opinion as well.
    >>      >     >
    >>      >     >     Thanks
    >>      >     >     Lokesh
    >>      >     >
    >>      >     >
    >>      >     >
    >>      >     >
    >>      >
    >>      >
    >>
    >>
    >>
    
    

Reply via email to