[GitHub] storm pull request #2483: (1.x) STORM-2870 Properly shutdown ExecutorService...

[arunmahadevan]

Github user arunmahadevan commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2483#discussion_r158759249
  
    --- Diff: 
storm-core/src/jvm/org/apache/storm/metric/FileBasedEventLogger.java ---
    @@ -60,8 +64,12 @@ private void initLogWriter(Path logFilePath) {
     
     
         private void setUpFlushTask() {
    -        ScheduledExecutorService scheduler = 
Executors.newSingleThreadScheduledExecutor();
    -        Runnable task = new Runnable() {
    +        ThreadFactory threadFactory = new ThreadFactoryBuilder()
    +                .setNameFormat("event-logger-flush-%d")
    --- End diff --
    
    should we add `setDaemon(true)` ?


---