incubator-blur-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Aaron McCurry <amccu...@gmail.com>
Subject Re: git commit: Fixed BLUR-337
Date Fri, 07 Mar 2014 02:44:24 GMT
On Thu, Mar 6, 2014 at 9:32 PM, Tim Williams <williamstw@gmail.com> wrote:

> On Thu, Mar 6, 2014 at 6:05 PM,  <amccurry@apache.org> wrote:
> > Repository: incubator-blur
> > Updated Branches:
> >   refs/heads/apache-blur-0.2 e0c1f6823 -> 97931740a
> >
> >
> > Fixed BLUR-337
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/incubator-blur/repo
> > Commit:
> http://git-wip-us.apache.org/repos/asf/incubator-blur/commit/97931740
> > Tree:
> http://git-wip-us.apache.org/repos/asf/incubator-blur/tree/97931740
> > Diff:
> http://git-wip-us.apache.org/repos/asf/incubator-blur/diff/97931740
> >
> > Branch: refs/heads/apache-blur-0.2
> > Commit: 97931740a4a80ba258750a3de463540be2b0110c
> > Parents: e0c1f68
> > Author: Aaron McCurry <amccurry@gmail.com>
> > Authored: Thu Mar 6 18:05:21 2014 -0500
> > Committer: Aaron McCurry <amccurry@gmail.com>
> > Committed: Thu Mar 6 18:05:21 2014 -0500
> >
> > ----------------------------------------------------------------------
> >  .../blur/thrift/ThriftBlurControllerServer.java | 25 ++++++++++-------
> >  .../blur/thrift/ThriftBlurShardServer.java      | 29
> ++++++++++++--------
> >  2 files changed, 32 insertions(+), 22 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/incubator-blur/blob/97931740/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
> > ----------------------------------------------------------------------
> > diff --git
> a/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
> b/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
> > index c0a8175..cd06b6b 100644
> > ---
> a/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
> > +++
> b/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurControllerServer.java
> > @@ -77,15 +77,20 @@ public class ThriftBlurControllerServer extends
> ThriftServer {
> >    private static final Log LOG =
> LogFactory.getLog(ThriftBlurControllerServer.class);
> >
> >    public static void main(String[] args) throws Exception {
> > -    int serverIndex = getServerIndex(args);
> > -    LOG.info("Setting up Controller Server");
> > -    BlurConfiguration configuration = new BlurConfiguration();
> > -    printUlimits();
> > -    ReporterSetup.setupReporters(configuration);
> > -    MemoryReporter.enable();
> > -    setupJvmMetrics();
> > -    ThriftServer server = createServer(serverIndex, configuration,
> false);
> > -    server.start();
> > +    try {
> > +      int serverIndex = getServerIndex(args);
> > +      LOG.info("Setting up Controller Server");
> > +      BlurConfiguration configuration = new BlurConfiguration();
> > +      printUlimits();
> > +      ReporterSetup.setupReporters(configuration);
> > +      MemoryReporter.enable();
> > +      setupJvmMetrics();
> > +      ThriftServer server = createServer(serverIndex, configuration,
> false);
> > +      server.start();
> > +    } catch (Throwable t) {
> > +      t.printStackTrace();
> > +      System.exit(1);
> > +    }
> >    }
> >
> >    public static ThriftServer createServer(int serverIndex,
> BlurConfiguration configuration, boolean randomPort)
> > @@ -182,7 +187,7 @@ public class ThriftBlurControllerServer extends
> ThriftServer {
> >      } else {
> >        httpServer = null;
> >      }
> > -
> > +
> >      if (httpServer != null) {
> >        WebAppContext context = httpServer.getContext();
> >        context.addServlet(new ServletHolder(new TServlet(new
> Blur.Processor<Blur.Iface>(iface),
> >
> >
> http://git-wip-us.apache.org/repos/asf/incubator-blur/blob/97931740/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
> > ----------------------------------------------------------------------
> > diff --git
> a/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
> b/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
> > index 7ee5f07..2c9fc92 100644
> > ---
> a/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
> > +++
> b/blur-core/src/main/java/org/apache/blur/thrift/ThriftBlurShardServer.java
> > @@ -108,18 +108,23 @@ public class ThriftBlurShardServer extends
> ThriftServer {
> >    private static final long _64MB = 64 * 1024 * 1024;
> >
> >    public static void main(String[] args) throws Exception {
> > -    int serverIndex = getServerIndex(args);
> > -    LOG.info("Setting up Shard Server");
> > -    Thread.setDefaultUncaughtExceptionHandler(new
> SimpleUncaughtExceptionHandler());
> > -    BlurConfiguration configuration = new BlurConfiguration();
> > -    printUlimits();
> > -    ReporterSetup.setupReporters(configuration);
> > -    MemoryReporter.enable();
> > -    setupJvmMetrics();
> > -    // make this configurable
> > -    GCWatcher.init(0.75);
> > -    ThriftServer server = createServer(serverIndex, configuration,
> false);
> > -    server.start();
> > +    try {
> > +      int serverIndex = getServerIndex(args);
> > +      LOG.info("Setting up Shard Server");
> > +      Thread.setDefaultUncaughtExceptionHandler(new
> SimpleUncaughtExceptionHandler());
> > +      BlurConfiguration configuration = new BlurConfiguration();
> > +      printUlimits();
> > +      ReporterSetup.setupReporters(configuration);
> > +      MemoryReporter.enable();
> > +      setupJvmMetrics();
> > +      // make this configurable
> > +      GCWatcher.init(0.75);
> > +      ThriftServer server = createServer(serverIndex, configuration,
> false);
> > +      server.start();
> > +    } catch (Throwable t) {
> > +      t.printStackTrace();
> > +      System.exit(1);
> > +    }
>
> I'm not that familiar with the
> Thread.setDefaultUncaughtExceptionHandler but, in general, it seems
> that addressing BLUR-337 would be more testable if it threw an
> exception rather than System.exit'ed?  In other words, we could define
> a bad configuration and "expected=RuntimeException.class" for things
> like ThriftServer.setupTraceStorage.... maybe we'd have to move the
> default exception handler to immediately before the catch block
> though.. thoughts?
>

The "Thread.setDefaultUncaughtExceptionHandler" is set so that any thread
(or pooled thread spawned by this thread, I think) will handle the
exception if it's thrown out of a newly executing thread.  So if a thread
pool throws a runtime exception, it's normally eaten.  This allows you to
handle it in some way.

As for the System.exit call, I did that because it was the easiest way for
the process to exit.  Because if there is a failure in the setup there are
various singletons that currently don't get cleaned up because of the way
we use the shutdown hook.  System.exit just guarantees that the process
will shutdown.

But don't think the default exception handler will help us here.

Aaron


>
> --tim
>

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