phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ankit Singhal <ankitsingha...@gmail.com>
Subject Re: [Discuss] Twill removal and Guava update plan
Date Tue, 09 Jun 2020 23:38:24 GMT
Thanks, Istvan for putting up a detailed plan

+1 for removing twill(as I can see it is recently moved to Attic) and
adding whatever convenience it's bringing in Omid/tephra project itself.

Instead of creating a shaded artifact of Guava and then refer it in Omid
and Tephra, can't we just short-circuit your step 2 with 3 and use
relocation of the maven-shaded plugin to produce the shaded artifacts for
Tephra/Omid as we are anyways shading everything including public API?



On Tue, Jun 2, 2020 at 8:25 AM Josh Elser <elserj@apache.org> wrote:

> Sounds like a well-thought-out plan to me.
>
> If we're going through and changing Guava, it may also be worthwhile to
> try to eliminate the use of Guava in our "public API". While the shaded
> guava eliminates classpath compatibility issues, Guava could (at any
> point) drop a class that we're using in our API and still break us. That
> could be a "later" thing.
>
> The only thing I think differently is that 4.x could (at some point)
> pick up the shaded guava artifact you describe and make the change.
> However, that's just for the future -- the call can be made if/when
> someone wants to do that :)
>
> On 6/2/20 10:01 AM, Istvan Toth wrote:
> > Hi!
> >
> > There are two related dependency issues that I believe should be solved
> in
> > Phoenix to keep it healthy and supportable.
> >
> > The Twill project has been officially terminated. Both Tephra and Omid
> > depend on it, and so transitively Phoenix does as well.
> >
> > Hadoop 3.3 has updated its Guava to 29, while Phoenix (master) is still
> on
> > 13.
> > None of Twill, Omid, Tephra, or Phoenix will run or compile against
> recent
> > Guava versions, which are pulled in by Hadoop 3.3.
> >
> > If we want to work with Hadoop 3.3, we either have to update all
> > dependencies to a recent Guava version, or we have to build our artifacts
> > with shaded Guava.
> > Since Guava 13 has known vulnerabilities, including in the classpath
> causes
> > a barrier to adaptation. Some potential Phoenix users consider including
> > dependencies with
> > known vulnerabilities a show-stopper, they do not care if the
> vulnerability
> > affects Phoenix or not.
> >
> > I propose that we take following steps to ensure compatibility with
> > upcoming Hadoop versions:
> >
> > *1. Remove the Twill dependency from Omid and Tephra*
> > It is generally not healthy to depend on abandoned projects, but the fact
> > Twill also depends (heavily) on Guava 13, makes removal the best
> solution.
> > As far as I can see, Omid and Tephra mostly use the ZK client from Twill,
> > as well as the (transitively included) Guava service model.
> > Refactoring to use the native ZK client, and to use the Guava service
> > classes directly shouldn't be too difficult.
> >
> > *2. Create a shaded guava artifact for Omid and Tephra*
> > Since Omid and Tephra needs to work with Hadoop2 and Hadoop3 (including
> the
> > upcoming Hadoop 3.3), which already pull in Guava, we need to use
> different
> > Guava internally.
> > (similar to the HBase-thirdparty solution, but we need a separate one).
> > This artifact could live under the Phoenix groupId, but we'll need to be
> > careful with the circular dependencies.
> >
> > *3. Update the Omid and Tephra to use the shaded Guava artifact*
> > Apart from handling the mostly trivial, "let's break API compatibility
> for
> > the heck of it" Guava changes, the Guava Service API that both Omid and
> > Tephra build on has changed significantly.
> > This will mean changes in the public (Phoenix facing) APIs. All Guava
> > references will have to be replaced with the shaded guava classes from
> step
> > 2.
> >
> > *3. Define self-contained public APIs for Omid and Tephra*
> > To break the public API's dependency on Guava, redefine the public APIs
> in
> > such a way that they do not have Guava classes as ancestors.
> > This doesn't mean that we decouple the internal implementation from
> Guava,
> > simply defining a set of java Interfaces that matches the existing
> (updated
> > to recent Guava Service API)
> > interface's signature, but is self-contained under the Tephra/Omid
> > namespace should do the trick.
> >
> > *4. Update Phoenix to use new Omid/Tephra API*
> > i.e. use the new Interface that we defined in step 3.
> >
> > *5. Update Phoenix to work with Guava 13-29.*
> > We need to somehow get Phoenix work with both old and new Guava.
> > Probably the least disruptive way to do this is reduce the Guava use to
> the
> > common subset of 13.0 and 29.0, and replace/reimplement the parts that
> > cannot be resolved.
> > Alternatively, we could rebase to the same shaded guava-thirdparty
> library
> > that we use for Omid and Tephra.
> >
> > For *4.x*, since we cannot get rid of Guava 13 ever, *Step 5 *is not
> > necessary
> >
> > I am very interested in your opinion on the above plan.
> > Does anyone have any objections ?
> > Does anyone have a better solution ?
> > Is there some hidden pitfall that I hadn't considered (there certainly
> > is...) ?
> >
> > best regards
> >
> > Istvan
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message