> 4. Missing information about increasing MVN memory. We could add the memory requirement in pom. I added the following in RATIS-237. It seems working.
./pom.xml + <meminitial>2048m</meminitial> + <maxmem>4096m</maxmem> Tsz-Wo On Tue, May 8, 2018 at 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 >>> > > >>> > > >>> > > >>> > > >>> > >>> > >>> >>> >>> >
