accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Christopher Tubbs" <ctubbsii...@apache.org>
Subject Re: Review Request 23988: Use Dropwizard to create a proper REST monitor service
Date Tue, 29 Jul 2014 19:02:44 GMT


> On July 28, 2014, 1:35 p.m., Christopher Tubbs wrote:
> > assemble/bin/start-all.sh, lines 62-63
> > <https://reviews.apache.org/r/23988/diff/1/?file=643615#file643615line62>
> >
> >     Since this component is optional, it seems like it should not be started automatically
with the other standard services (the ones users have come to expect).
> 
> Josh Elser wrote:
>     That depends. If the intent is still for the monitor (as we know it now) to eventually
use this service (make the monitor a consumer of this rest service), I think it makes sense
to just start the process and get people used to it.

Is it possible to spawn this as part of the monitor, rather than a separate service, then?
I don't mind a separate lib, but it seems a bit weird to have a separate process.


> On July 28, 2014, 1:35 p.m., Christopher Tubbs wrote:
> > pom.xml, line 129
> > <https://reviews.apache.org/r/23988/diff/1/?file=643620#file643620line129>
> >
> >     Are we introducing incompatibilities that will make it harder for users to use
newer versions of Jetty? Why the version drop?
> 
> Josh Elser wrote:
>     Dropwizard bundles this all for us, and thus, to use it, requires the drop to make
it work. Users presently don't care what version of Jetty is used. The point of the library
isn't to make a war file that can be deployed but to make a singular artifact that run the
service. If you're referring to system integrators (like what you're doing for fedora), the
point is still moot because you don't need jetty installed on the system elsewhere. The dependencies
are encapsulated within the monitor-rest artifact.

The point isn't moot. Many distributions disallow bundled binaries (because by some definitions,
such are not considered "open source" even if they were derived from open source). And, forcing
this to be a dependency (using the shaded jar approach) is a non-starter for such requirements.
The approach to deal with bundled binaries is to rebuild from source... but that can't be
done if it depends on an old version which has been superceded.

That's one reason why one might care what version of Jetty is being used. Another (quite important
reason) why one might care is because an older version might have security vulnerabilities
or bugs that require a user to use a newer version. If they cannot use a newer version, because
we depend on something that exists in an older version (which happens routinely anyway, but
it's still good to avoid if possible), that makes it that much harder for a user to adopt
the software.

For my packaging requirements in Fedora, it doesn't really matter, because I simply won't
build the REST service if it requires a version not available. It's an optional component
(as implemented), so it wouldn't affect me much to exclude it.


> On July 28, 2014, 1:35 p.m., Christopher Tubbs wrote:
> > server/monitor-rest/pom.xml, lines 98-131
> > <https://reviews.apache.org/r/23988/diff/1/?file=643622#file643622line98>
> >
> >     Please don't shade by default in the build. It creates a nightmare for pom dependency
resolution. We should not be shipping shaded binary artifacts in a release, or deploying them
to maven central in a release.
> 
> Josh Elser wrote:
>     That's the whole point of Dropwizard. I'd recommend you read into it - https://dropwizard.github.io/dropwizard/getting-started.html,
specifically https://dropwizard.github.io/dropwizard/getting-started.html#building-fat-jars.
> 
> Dave Marion wrote:
>     Consumers of the rest service may need the client classes depending on what they
are doing in their application. In this case they will have to use the shaded jar, as it contains
the client classes, and it will also pull in the jax-rs, jax-b, and jackson jars which may
conflict with what their application is doing. If we have a shaded jar on the server side,
for ease of classpath or whatever, then I think we want to create a client jar that only contains
the client classes.
> 
> Josh Elser wrote:
>     That's valid. The pojos could be split into their own artifact (long term, probably
rolled into the long talked about 'accumulo-client' jar or similar).

Shading comes with a whole host of problems, some of which I mentioned above, but also because
anybody having a dependency on some POJO inside the jar is going to have a huge nightmare
with additional bundled stuff.

If that's the only way to reasonable work with dropwizard, then I think we should look at
alternative REST frameworks, for something that can be more easily baked in to the existing
monitor packaging. Alternatively, we could do a full separation and provide that service as
a separate project or contrib, which might actually help us focus on providing a standard
API for metrics/stats that any monitoring tool might be able to use, rather than enriching
the existing one.

On the other hand, that link you provide doesn't really explain that we need to build shaded
jars... it just explains what shaded jars are and why you might want to create them. In our
case, I think non-shaded is preferred. It fits better into our existing build and scripts,
and doesn't come with all the problems that shaded jars have.


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23988/#review48869
-----------------------------------------------------------


On July 28, 2014, 12:55 p.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23988/
> -----------------------------------------------------------
> 
> (Updated July 28, 2014, 12:55 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3005
>     https://issues.apache.org/jira/browse/ACCUMULO-3005
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Creates a proper REST service using Dropwizard, with the intent to eventually replace
the existing monitor's "data" component. Copies most of the functionality (sans the log-forwarding)
into a standalone application. Returns data as JSON and tries to separate logic into consumable
pieces. Still uses the Monitor class for most Thrift interactions.
> 
> 
> Diffs
> -----
> 
>   assemble/bin/accumulo 727a4c8 
>   assemble/bin/start-all.sh cebbd8c 
>   assemble/bin/stop-all.sh 4bf06c0 
>   assemble/bin/stop-server.sh 52696af 
>   assemble/conf/templates/accumulo-site.xml 08c905b 
>   assemble/pom.xml 89a3747 
>   pom.xml ba6693d 
>   server/monitor-rest/.gitignore PRE-CREATION 
>   server/monitor-rest/pom.xml PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/MonitorApplication.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/MonitorConfiguration.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/GarbageCollection.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/GarbageCollectorCycle.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/GarbageCollectorStatus.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/LogEvent.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/RecoveryStatusInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/ReplicationInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/TableInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/TabletServerInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/TabletServerTableInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/api/TabletServerWithTableInformation.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/health/AccumuloHealthCheck.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/GarbageCollectorResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/LogResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/MasterResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/ProblemsResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/ReplicationResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/StatisticsOverTimeResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/StatisticsResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/TablesResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/java/org/apache/accumulo/monitor/rest/resources/TabletServerResource.java
PRE-CREATION 
>   server/monitor-rest/src/main/resources/accumulo-monitor-rest.yaml PRE-CREATION 
>   server/monitor-rest/src/test/resources/log4j.properties PRE-CREATION 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java 268516c 
> 
> Diff: https://reviews.apache.org/r/23988/diff/
> 
> 
> Testing
> -------
> 
> Tested against a single-node Accumulo instance. While this code is much easier to test,
I haven't written new unit tests yet.
> 
> 
> Thanks,
> 
> Josh Elser
> 
>


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