Merge pull request #3939 from fujimotos/sf/win32-logrotate

Fix "unable to rotate log files on Windows" bug

lib/fluent/command/fluentd.rb

@@ -345,17 +345,6 @@
 exit 0 if early_exit
 
 if opts[:supervise]
-  if Fluent.windows?
-    if opts[:log_path] && opts[:log_path] != "-"
-      if opts[:log_rotate_age] || opts[:log_rotate_size]
-        require 'pathname'
-
-        log_path = Pathname(opts[:log_path]).sub_ext("-supervisor#{Pathname(opts[:log_path]).extname}").to_s
-        opts[:log_path] = log_path
-      end
-    end
-  end
-
   supervisor = Fluent::Supervisor.new(opts)
   supervisor.configure(supervisor: true)
   supervisor.run_supervisor(dry_run: opts[:dry_run])

lib/fluent/supervisor.rb

@@ -525,10 +525,22 @@ def initialize(path, level, chuser, chgroup, opts, log_rotate_age: nil, log_rota
         @log_rotate_size = log_rotate_size
       end
 
-      def worker_id_suffixed_path(worker_id, path)
-        require 'pathname'
-
-        Pathname(path).sub_ext("-#{worker_id}#{Pathname(path).extname}").to_s
+      # Create a unique path for each process.
+      #
+      # >>> per_process_path(:worker, 1, "C:/tmp/test.log")
+      # C:/tmp/test-1.log
+      # >>> per_process_path(:supervisor, 0, "C:/tmp/test.log")
+      # C:/tmp/test-supervisor-0.log
+      def self.per_process_path(path, process_type, worker_id)
+        path = Pathname(path)
+        ext = path.extname
+
+        if process_type == :supervisor
+          suffix = "-#{process_type}-0#{ext}"  # "-0" for backword compatibility.
+        else
+          suffix = "-#{worker_id}#{ext}"
+        end
+        return path.sub_ext(suffix).to_s
       end
 
       def init(process_type, worker_id)
@@ -540,13 +552,19 @@ def init(process_type, worker_id)
             FileUtils.mkdir_p(File.dirname(@path))
           end
 
-          @logdev = if @log_rotate_age || @log_rotate_size
-                     Fluent::LogDeviceIO.new(Fluent.windows? ?
-                                               worker_id_suffixed_path(worker_id, @path) : @path,
-                                             shift_age: @log_rotate_age, shift_size: @log_rotate_size)
-                   else
-                     File.open(@path, "a")
-                   end
+          if @log_rotate_age || @log_rotate_size
+            # We need to prepare a unique path for each worker since
+            # Windows locks files.
+            if Fluent.windows?
+                path = LoggerInitializer.per_process_path(@path, process_type, worker_id)
+            else
+                path = @path
+            end
+            @logdev = Fluent::LogDeviceIO.new(path, shift_age: @log_rotate_age, shift_size: @log_rotate_size)
+          else
+            @logdev = File.open(@path, "a")
+          end
+
           if @chuser || @chgroup
             chuid = @chuser ? ServerEngine::Privilege.get_etc_passwd(@chuser).uid : nil
             chgid = @chgroup ? ServerEngine::Privilege.get_etc_group(@chgroup).gid : nil
@@ -565,6 +583,7 @@ def init(process_type, worker_id)
         $log = Fluent::Log.new(logger, @opts)
         $log.enable_color(false) if @path
         $log.enable_debug if @level <= Fluent::Log::LEVEL_DEBUG
+        $log.info "init #{process_type} logger", path: path, rotate_age: @log_rotate_age, rotate_size: @log_rotate_size
       end
 
       def stdout?

test/test_supervisor.rb

@@ -787,6 +787,14 @@ def server.config
     end
   end
 
+  def test_per_process_path
+    path = Fluent::Supervisor::LoggerInitializer.per_process_path("C:/tmp/test.log", :supervisor, 0)
+    assert_equal(path, "C:/tmp/test-supervisor-0.log")
+
+    path = Fluent::Supervisor::LoggerInitializer.per_process_path("C:/tmp/test.log", :worker, 1)
+    assert_equal(path, "C:/tmp/test-1.log")
+  end
+
   def create_debug_dummy_logger
     dl_opts = {}
     dl_opts[:log_level] = ServerEngine::DaemonLogger::DEBUG