Re: Review Request 33456: Adding logging threadpool executor.

[Maxim Khutornenko]

> On April 27, 2015, 8 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java, line 71
> > 
> >
> > Have you considered creating factory methods that apply decorators to 
> > ExecutorServices?  That would potentially save this code from the 
> > combinatoric explosion we seem to be heading towards.  
> > ForwardingExecutorService [1] could be helpful to minimize the code to 
> > implement decorators.
> > 
> > [1] 
> > http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/ForwardingExecutorService.html
> 
> Maxim Khutornenko wrote:
> I could not figure out how to make it useful for this particular case. 
> The `afterExecute` is defined in a more concrete ThreadPoolExecutor rather 
> than in ExecutorService that ForwardingExecutorService implements. The 
> implementation does not leave a second chance to handle unhandled error 
> (ThreadPoolExecutor.runWorker):
> ```
> try {
> beforeExecute(wt, task);
> Throwable thrown = null;
> try {
> task.run();
> } catch (RuntimeException x) {
> thrown = x; throw x;
> } catch (Error x) {
> thrown = x; throw x;
> } catch (Throwable x) {
> thrown = x; throw new Error(x);
> } finally {
> afterExecute(task, thrown);
> }
> } finally {
> task = null;
> w.completedTasks++;
> w.unlock();
> }
> ```
> 
> Bill Farner wrote:
> Darn.  Thinking more broadly about this, it's surprising that we're 
> getting no logging at all.  We should at least have a default uncaught 
> exception handler installed that logs [1].
> 
> Thinking from that direction, it's obvious why this is happening.  We 
> create these thread pools before the uncaught exception handler is installed. 
>  I suggest we fix that instead - define a custom `UncaughtExceptionHandler`, 
> and call `Thread.setDefaultUncaughtExceptionHandler()` as the first order of 
> business in `main()` [2].
> 
> 
> [1] 
> https://github.com/twitter/commons/blob/9fc05c8cc927ade33cb14ee21d433669af218c91/src/java/com/twitter/common/application/Lifecycle.java#L48
> [2] 
> https://github.com/apache/aurora/blob/65df91bfd7e3a2ada38a5fe4d620e6373d0f59bf/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java#L263
> 
> Bill Farner wrote:
> Oh wait, i forgot that does not fully address the problem when the 
> ExecutorService catches the exception internally.  We should still address 
> the above issue, but it's clearly not a solution by itself.
> 
> Bill Farner wrote:
> Going back to my first suggestion above, can you not wrap the incoming 
> Runnables and Callables?
> 
> For example, the `ExecutorService` decorator might look like this:
> ```
>   public static ExecutorService logExceptions(final ExecutorService 
> service) {
> return new ForwardingExecutorService() {
>   @Override
>   protected ExecutorService delegate() {
> return service;
>   }
> 
>   private Runnable logExceptions(final Runnable command) {
> return new Runnable() {
>   @Override
>   public void run() {
> try {
>   command.run();
> } catch (RuntimeException e) {
>   // Log.
> }
>   }
> };
>   }
> 
>   @Override
>   public void execute(Runnable command) {
> super.execute(logExceptions(command));
>   }
> };
>   }
> ```
> 
> There's a bunch of methods to override, but `logExceptions` and a 
> counterpart for `Callable` would be reusable, and we don't have have the 
> factory method explosion.
> 
> Maxim Khutornenko wrote:
> This may work for the ExecutorService but will not wrap the 
> ScheduledExecutorService.submit(), which is defined in downstream.
> 
> This is the first and likely the last entry in this class as we now have 
> both scheduled and regular executors represented here, which seems to cover 
> all our use cases. So, I am not sure I share your concern about combinatorial 
> explosion.

I have played with a few different approaches and neither produced a better 
outcome that what we have now. There is no ForwardingScheduledExecutorService 
available off the shelf, so we would have to implement our own with a myriad of 
methods to forward including those we will never use. I also don't like 
creatin

Re: Review Request 33456: Adding logging threadpool executor.

[Bill Farner]

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

Ship it!


I'll let you make the call here.  I think the decorator methods decouple us 
from the implementations of [Scheduled]ExecutorService, and the code to do it 
is very mechanical.  However, it will likely require ~200 lines of this wiring 
code, which i can understand is undesirable at present.

- Bill Farner


On April 22, 2015, 10:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33456/
> ---
> 
> (Updated April 22, 2015, 10:58 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Our async EventBus is using a regular executor thus potentially hiding 
> unhandled errors.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
> f657e057b5bbff69971876e104ff0e47b2dc4faa 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> 3a4d40adc1abe170b5b80644db9f079751d8a9bf 
>   src/test/java/org/apache/aurora/scheduler/base/AsyncUtilTest.java 
> e990f528aac768b5c9b829c9544045a831e094fe 
> 
> Diff: https://reviews.apache.org/r/33456/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 33456: Adding logging threadpool executor.

[Maxim Khutornenko]

> On April 27, 2015, 8 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java, line 71
> > 
> >
> > Have you considered creating factory methods that apply decorators to 
> > ExecutorServices?  That would potentially save this code from the 
> > combinatoric explosion we seem to be heading towards.  
> > ForwardingExecutorService [1] could be helpful to minimize the code to 
> > implement decorators.
> > 
> > [1] 
> > http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/ForwardingExecutorService.html
> 
> Maxim Khutornenko wrote:
> I could not figure out how to make it useful for this particular case. 
> The `afterExecute` is defined in a more concrete ThreadPoolExecutor rather 
> than in ExecutorService that ForwardingExecutorService implements. The 
> implementation does not leave a second chance to handle unhandled error 
> (ThreadPoolExecutor.runWorker):
> ```
> try {
> beforeExecute(wt, task);
> Throwable thrown = null;
> try {
> task.run();
> } catch (RuntimeException x) {
> thrown = x; throw x;
> } catch (Error x) {
> thrown = x; throw x;
> } catch (Throwable x) {
> thrown = x; throw new Error(x);
> } finally {
> afterExecute(task, thrown);
> }
> } finally {
> task = null;
> w.completedTasks++;
> w.unlock();
> }
> ```
> 
> Bill Farner wrote:
> Darn.  Thinking more broadly about this, it's surprising that we're 
> getting no logging at all.  We should at least have a default uncaught 
> exception handler installed that logs [1].
> 
> Thinking from that direction, it's obvious why this is happening.  We 
> create these thread pools before the uncaught exception handler is installed. 
>  I suggest we fix that instead - define a custom `UncaughtExceptionHandler`, 
> and call `Thread.setDefaultUncaughtExceptionHandler()` as the first order of 
> business in `main()` [2].
> 
> 
> [1] 
> https://github.com/twitter/commons/blob/9fc05c8cc927ade33cb14ee21d433669af218c91/src/java/com/twitter/common/application/Lifecycle.java#L48
> [2] 
> https://github.com/apache/aurora/blob/65df91bfd7e3a2ada38a5fe4d620e6373d0f59bf/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java#L263
> 
> Bill Farner wrote:
> Oh wait, i forgot that does not fully address the problem when the 
> ExecutorService catches the exception internally.  We should still address 
> the above issue, but it's clearly not a solution by itself.
> 
> Bill Farner wrote:
> Going back to my first suggestion above, can you not wrap the incoming 
> Runnables and Callables?
> 
> For example, the `ExecutorService` decorator might look like this:
> ```
>   public static ExecutorService logExceptions(final ExecutorService 
> service) {
> return new ForwardingExecutorService() {
>   @Override
>   protected ExecutorService delegate() {
> return service;
>   }
> 
>   private Runnable logExceptions(final Runnable command) {
> return new Runnable() {
>   @Override
>   public void run() {
> try {
>   command.run();
> } catch (RuntimeException e) {
>   // Log.
> }
>   }
> };
>   }
> 
>   @Override
>   public void execute(Runnable command) {
> super.execute(logExceptions(command));
>   }
> };
>   }
> ```
> 
> There's a bunch of methods to override, but `logExceptions` and a 
> counterpart for `Callable` would be reusable, and we don't have have the 
> factory method explosion.

This may work for the ExecutorService but will not wrap the 
ScheduledExecutorService.submit(), which is defined in downstream.

This is the first and likely the last entry in this class as we now have both 
scheduled and regular executors represented here, which seems to cover all our 
use cases. So, I am not sure I share your concern about combinatorial explosion.


- Maxim


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


On April 22, 2015, 10:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is a

Re: Review Request 33456: Adding logging threadpool executor.

[Bill Farner]

> On April 27, 2015, 8 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java, line 71
> > 
> >
> > Have you considered creating factory methods that apply decorators to 
> > ExecutorServices?  That would potentially save this code from the 
> > combinatoric explosion we seem to be heading towards.  
> > ForwardingExecutorService [1] could be helpful to minimize the code to 
> > implement decorators.
> > 
> > [1] 
> > http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/ForwardingExecutorService.html
> 
> Maxim Khutornenko wrote:
> I could not figure out how to make it useful for this particular case. 
> The `afterExecute` is defined in a more concrete ThreadPoolExecutor rather 
> than in ExecutorService that ForwardingExecutorService implements. The 
> implementation does not leave a second chance to handle unhandled error 
> (ThreadPoolExecutor.runWorker):
> ```
> try {
> beforeExecute(wt, task);
> Throwable thrown = null;
> try {
> task.run();
> } catch (RuntimeException x) {
> thrown = x; throw x;
> } catch (Error x) {
> thrown = x; throw x;
> } catch (Throwable x) {
> thrown = x; throw new Error(x);
> } finally {
> afterExecute(task, thrown);
> }
> } finally {
> task = null;
> w.completedTasks++;
> w.unlock();
> }
> ```
> 
> Bill Farner wrote:
> Darn.  Thinking more broadly about this, it's surprising that we're 
> getting no logging at all.  We should at least have a default uncaught 
> exception handler installed that logs [1].
> 
> Thinking from that direction, it's obvious why this is happening.  We 
> create these thread pools before the uncaught exception handler is installed. 
>  I suggest we fix that instead - define a custom `UncaughtExceptionHandler`, 
> and call `Thread.setDefaultUncaughtExceptionHandler()` as the first order of 
> business in `main()` [2].
> 
> 
> [1] 
> https://github.com/twitter/commons/blob/9fc05c8cc927ade33cb14ee21d433669af218c91/src/java/com/twitter/common/application/Lifecycle.java#L48
> [2] 
> https://github.com/apache/aurora/blob/65df91bfd7e3a2ada38a5fe4d620e6373d0f59bf/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java#L263
> 
> Bill Farner wrote:
> Oh wait, i forgot that does not fully address the problem when the 
> ExecutorService catches the exception internally.  We should still address 
> the above issue, but it's clearly not a solution by itself.

Going back to my first suggestion above, can you not wrap the incoming 
Runnables and Callables?

For example, the `ExecutorService` decorator might look like this:
```
  public static ExecutorService logExceptions(final ExecutorService service) {
return new ForwardingExecutorService() {
  @Override
  protected ExecutorService delegate() {
return service;
  }

  private Runnable logExceptions(final Runnable command) {
return new Runnable() {
  @Override
  public void run() {
try {
  command.run();
} catch (RuntimeException e) {
  // Log.
}
  }
};
  }

  @Override
  public void execute(Runnable command) {
super.execute(logExceptions(command));
  }
};
  }
```

There's a bunch of methods to override, but `logExceptions` and a counterpart 
for `Callable` would be reusable, and we don't have have the factory method 
explosion.


- Bill


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


On April 22, 2015, 10:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33456/
> ---
> 
> (Updated April 22, 2015, 10:58 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Our async EventBus is using a regular executor thus potentially hiding 
> unhandled errors.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
> f657e057b5bbff69971876e104ff0e47b2dc4faa 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventMod

Re: Review Request 33456: Adding logging threadpool executor.

[Bill Farner]

> On April 27, 2015, 8 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java, line 71
> > 
> >
> > Have you considered creating factory methods that apply decorators to 
> > ExecutorServices?  That would potentially save this code from the 
> > combinatoric explosion we seem to be heading towards.  
> > ForwardingExecutorService [1] could be helpful to minimize the code to 
> > implement decorators.
> > 
> > [1] 
> > http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/ForwardingExecutorService.html
> 
> Maxim Khutornenko wrote:
> I could not figure out how to make it useful for this particular case. 
> The `afterExecute` is defined in a more concrete ThreadPoolExecutor rather 
> than in ExecutorService that ForwardingExecutorService implements. The 
> implementation does not leave a second chance to handle unhandled error 
> (ThreadPoolExecutor.runWorker):
> ```
> try {
> beforeExecute(wt, task);
> Throwable thrown = null;
> try {
> task.run();
> } catch (RuntimeException x) {
> thrown = x; throw x;
> } catch (Error x) {
> thrown = x; throw x;
> } catch (Throwable x) {
> thrown = x; throw new Error(x);
> } finally {
> afterExecute(task, thrown);
> }
> } finally {
> task = null;
> w.completedTasks++;
> w.unlock();
> }
> ```
> 
> Bill Farner wrote:
> Darn.  Thinking more broadly about this, it's surprising that we're 
> getting no logging at all.  We should at least have a default uncaught 
> exception handler installed that logs [1].
> 
> Thinking from that direction, it's obvious why this is happening.  We 
> create these thread pools before the uncaught exception handler is installed. 
>  I suggest we fix that instead - define a custom `UncaughtExceptionHandler`, 
> and call `Thread.setDefaultUncaughtExceptionHandler()` as the first order of 
> business in `main()` [2].
> 
> 
> [1] 
> https://github.com/twitter/commons/blob/9fc05c8cc927ade33cb14ee21d433669af218c91/src/java/com/twitter/common/application/Lifecycle.java#L48
> [2] 
> https://github.com/apache/aurora/blob/65df91bfd7e3a2ada38a5fe4d620e6373d0f59bf/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java#L263

Oh wait, i forgot that does not fully address the problem when the 
ExecutorService catches the exception internally.  We should still address the 
above issue, but it's clearly not a solution by itself.


- Bill


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


On April 22, 2015, 10:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33456/
> ---
> 
> (Updated April 22, 2015, 10:58 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Our async EventBus is using a regular executor thus potentially hiding 
> unhandled errors.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
> f657e057b5bbff69971876e104ff0e47b2dc4faa 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> 3a4d40adc1abe170b5b80644db9f079751d8a9bf 
>   src/test/java/org/apache/aurora/scheduler/base/AsyncUtilTest.java 
> e990f528aac768b5c9b829c9544045a831e094fe 
> 
> Diff: https://reviews.apache.org/r/33456/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 33456: Adding logging threadpool executor.

[Bill Farner]

> On April 27, 2015, 8 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java, line 71
> > 
> >
> > Have you considered creating factory methods that apply decorators to 
> > ExecutorServices?  That would potentially save this code from the 
> > combinatoric explosion we seem to be heading towards.  
> > ForwardingExecutorService [1] could be helpful to minimize the code to 
> > implement decorators.
> > 
> > [1] 
> > http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/ForwardingExecutorService.html
> 
> Maxim Khutornenko wrote:
> I could not figure out how to make it useful for this particular case. 
> The `afterExecute` is defined in a more concrete ThreadPoolExecutor rather 
> than in ExecutorService that ForwardingExecutorService implements. The 
> implementation does not leave a second chance to handle unhandled error 
> (ThreadPoolExecutor.runWorker):
> ```
> try {
> beforeExecute(wt, task);
> Throwable thrown = null;
> try {
> task.run();
> } catch (RuntimeException x) {
> thrown = x; throw x;
> } catch (Error x) {
> thrown = x; throw x;
> } catch (Throwable x) {
> thrown = x; throw new Error(x);
> } finally {
> afterExecute(task, thrown);
> }
> } finally {
> task = null;
> w.completedTasks++;
> w.unlock();
> }
> ```

Darn.  Thinking more broadly about this, it's surprising that we're getting no 
logging at all.  We should at least have a default uncaught exception handler 
installed that logs [1].

Thinking from that direction, it's obvious why this is happening.  We create 
these thread pools before the uncaught exception handler is installed.  I 
suggest we fix that instead - define a custom `UncaughtExceptionHandler`, and 
call `Thread.setDefaultUncaughtExceptionHandler()` as the first order of 
business in `main()` [2].


[1] 
https://github.com/twitter/commons/blob/9fc05c8cc927ade33cb14ee21d433669af218c91/src/java/com/twitter/common/application/Lifecycle.java#L48
[2] 
https://github.com/apache/aurora/blob/65df91bfd7e3a2ada38a5fe4d620e6373d0f59bf/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java#L263


- Bill


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


On April 22, 2015, 10:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33456/
> ---
> 
> (Updated April 22, 2015, 10:58 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Our async EventBus is using a regular executor thus potentially hiding 
> unhandled errors.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
> f657e057b5bbff69971876e104ff0e47b2dc4faa 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> 3a4d40adc1abe170b5b80644db9f079751d8a9bf 
>   src/test/java/org/apache/aurora/scheduler/base/AsyncUtilTest.java 
> e990f528aac768b5c9b829c9544045a831e094fe 
> 
> Diff: https://reviews.apache.org/r/33456/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 33456: Adding logging threadpool executor.

[Maxim Khutornenko]

> On April 27, 2015, 8 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java, line 71
> > 
> >
> > Have you considered creating factory methods that apply decorators to 
> > ExecutorServices?  That would potentially save this code from the 
> > combinatoric explosion we seem to be heading towards.  
> > ForwardingExecutorService [1] could be helpful to minimize the code to 
> > implement decorators.
> > 
> > [1] 
> > http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/ForwardingExecutorService.html

I could not figure out how to make it useful for this particular case. The 
`afterExecute` is defined in a more concrete ThreadPoolExecutor rather than in 
ExecutorService that ForwardingExecutorService implements. The implementation 
does not leave a second chance to handle unhandled error 
(ThreadPoolExecutor.runWorker):
```
try {
beforeExecute(wt, task);
Throwable thrown = null;
try {
task.run();
} catch (RuntimeException x) {
thrown = x; throw x;
} catch (Error x) {
thrown = x; throw x;
} catch (Throwable x) {
thrown = x; throw new Error(x);
} finally {
afterExecute(task, thrown);
}
} finally {
task = null;
w.completedTasks++;
w.unlock();
}
```


- Maxim


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


On April 22, 2015, 10:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33456/
> ---
> 
> (Updated April 22, 2015, 10:58 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Our async EventBus is using a regular executor thus potentially hiding 
> unhandled errors.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
> f657e057b5bbff69971876e104ff0e47b2dc4faa 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> 3a4d40adc1abe170b5b80644db9f079751d8a9bf 
>   src/test/java/org/apache/aurora/scheduler/base/AsyncUtilTest.java 
> e990f528aac768b5c9b829c9544045a831e094fe 
> 
> Diff: https://reviews.apache.org/r/33456/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 33456: Adding logging threadpool executor.

[Bill Farner]

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



src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java


Have you considered creating factory methods that apply decorators to 
ExecutorServices?  That would potentially save this code from the combinatoric 
explosion we seem to be heading towards.  ForwardingExecutorService [1] could 
be helpful to minimize the code to implement decorators.

[1] 
http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/ForwardingExecutorService.html


- Bill Farner


On April 22, 2015, 10:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33456/
> ---
> 
> (Updated April 22, 2015, 10:58 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Our async EventBus is using a regular executor thus potentially hiding 
> unhandled errors.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
> f657e057b5bbff69971876e104ff0e47b2dc4faa 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> 3a4d40adc1abe170b5b80644db9f079751d8a9bf 
>   src/test/java/org/apache/aurora/scheduler/base/AsyncUtilTest.java 
> e990f528aac768b5c9b829c9544045a831e094fe 
> 
> Diff: https://reviews.apache.org/r/33456/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 33456: Adding logging threadpool executor.

[Aurora ReviewBot]

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

Ship it!


Master (352e0ef) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On April 22, 2015, 10:58 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33456/
> ---
> 
> (Updated April 22, 2015, 10:58 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Our async EventBus is using a regular executor thus potentially hiding 
> unhandled errors.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 
> f657e057b5bbff69971876e104ff0e47b2dc4faa 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 
> 3a4d40adc1abe170b5b80644db9f079751d8a9bf 
>   src/test/java/org/apache/aurora/scheduler/base/AsyncUtilTest.java 
> e990f528aac768b5c9b829c9544045a831e094fe 
> 
> Diff: https://reviews.apache.org/r/33456/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>