accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 23988: Use Dropwizard to create a proper REST monitor service
Date Tue, 29 Jul 2014 19:24:06 GMT


> On July 28, 2014, 5: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.
> 
> Christopher Tubbs wrote:
>     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.

Presently, no. Long term, the monitor process would go away and we'd move the existing "monitor"
into some templated views. But, that's out of the scope of what I was initially trying to
do here. I haven't entirely looked into how to do that yet, either.


> On July 28, 2014, 5: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).
> 
> Christopher Tubbs wrote:
>     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.

Won't restate the POJO resolution as I already addressed that.

The reason I like the recommendation of the shaded jar is because no one on this project is
a real "expert" on setting up Java web services. It gets out of the way to provide some easy
standards of just writing code. I do like the technology as well (Jackson, Jetty, and Jersey),
but boy do I hate trying to actually set it up by hand for numerous reasons (primarily death
by 1000 configuration parameters).

Removing the shade might not be *too* bad because we could hide the dependencies necessary
via the assembly pom and the shell scripts (which is the argument that Dropwizard typically
makes - you just need a single jar that can be deployed with a simple `java -jar foo.jar server`
and not having to futz with the classpath). Having our own collection of scripts will mitigate
some of the pains that shading solves. I'll see if I can get something working without shade
some evening.

Integration with metrics systems is outside of the scope of what this is providing, but there
is already integration with JMX as well as Ganglia. Both of those are also unrelated to these
changes. While yes, it may be nice if we could integrate with some magical metrics library
that gives us everything we want, I've yet to find such a thing. Until then, it's pointless
to keep an unmaintainable collection of code just because it "would be nice" if such a magical
library existed. This diff is making improvements to Accumulo providing metrics data in a
consumable format.

The splitting out of the existing servlet classes into data producers (REST endpoints), we
already make one step closer to easing integration with other systems. Additionally, creating
POJOs for the data being returned also allows these integrations to more reliably use the
data we produce.


- Josh


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


On July 28, 2014, 4: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, 4: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