Skip to content

Commit fc76fec

Browse files
committed
in_tail: queue oversized batches on workers
Signed-off-by: Takuro Ashie <ashie@clear-code.com>
1 parent 1fe62b9 commit fc76fec

4 files changed

Lines changed: 94 additions & 10 deletions

File tree

‎lib/fluent/plugin/in_tail.rb‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,7 @@ class TailInput
816816
none regexp json csv tsv ltsv msgpack apache apache2 apache_error nginx syslog
817817
].freeze
818818
WORKER_QUEUE_BYTES = 64 * 1024 * 1024
819-
WorkerBatch = Struct.new(:watcher, :lines, :fallback_lines)
819+
WorkerBatch = Struct.new(:watcher, :lines)
820820

821821
private
822822

@@ -866,7 +866,7 @@ def setup_worker_pool
866866
thread_create: ->(title, &block) { Thread.new { Thread.current.name = title.to_s if Thread.current.respond_to?(:name=); block.call } },
867867
notify: @worker_completion_watcher.method(:signal)
868868
) do |index, batch|
869-
@worker_line_feeders[index].parse(batch.lines, batch.watcher)
869+
@worker_line_feeders[index].parse(batch.lines.map(&:dup), batch.watcher)
870870
end
871871
@worker_pool.start
872872
rescue
@@ -892,12 +892,8 @@ def build_worker_line_feeder(parser)
892892
def async_receive_lines(lines, watcher)
893893
return @line_feeder.feed_lines(lines, watcher) unless @worker_pool.statistics[:state] == :running
894894

895-
worker_lines = lines.map(&:dup)
896-
fallback_lines = lines.map(&:dup)
897895
bytes = lines.sum(&:bytesize) * 3
898-
return @line_feeder.feed_lines(lines, watcher) if bytes > WORKER_QUEUE_BYTES
899-
900-
batch = WorkerBatch.new(watcher, worker_lines, fallback_lines)
896+
batch = WorkerBatch.new(watcher, lines)
901897
if @worker_pool.submit(watcher, batch, bytes: bytes)
902898
update_worker_metrics
903899
@worker_waiting_watchers.delete(watcher)
@@ -914,7 +910,7 @@ def process_worker_completions
914910
if result.error
915911
@worker_metrics[:parse_errors].inc
916912
log.warn 'worker parsing failed; retrying synchronously', path: batch.watcher.path, error: result.error
917-
emitted = @line_feeder.feed_lines(batch.fallback_lines, batch.watcher)
913+
emitted = @line_feeder.feed_lines(batch.lines, batch.watcher)
918914
else
919915
emitted = @line_feeder.emit(result.value, batch.watcher)
920916
end

‎lib/fluent/plugin/in_tail/worker_pool.rb‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ def initialize(num_threads:, task_limit:, byte_limit:, thread_create:, notify:,
4141
@results = Queue.new
4242
@active = {}
4343
@bytes = 0
44+
@regular_bytes = 0
45+
@oversized_task = nil
4446
@threads = []
4547
@state = :new
4648
@failure = nil
@@ -75,11 +77,22 @@ def submit(key, payload, bytes:)
7577
@mutex.synchronize do
7678
return false unless @state == :running
7779
return false if @active.key?(key) || @active.size >= @task_limit
78-
return false if @bytes + bytes > @byte_limit
80+
# A single over-limit batch may use an extra reservation alongside the
81+
# bounded regular budget so it does not force parsing on the event loop.
82+
if bytes > @byte_limit
83+
return false if @oversized_task
84+
else
85+
return false if @regular_bytes + bytes > @byte_limit
86+
end
7987

8088
task = Task.new(key, bytes, payload).freeze
8189
@active[key] = task
8290
@bytes += bytes
91+
if bytes > @byte_limit
92+
@oversized_task = task
93+
else
94+
@regular_bytes += bytes
95+
end
8396
@jobs << task
8497
true
8598
end
@@ -100,6 +113,11 @@ def acknowledge(result)
100113

101114
@active.delete(task.key)
102115
@bytes -= task.bytes
116+
if task.bytes > @byte_limit
117+
@oversized_task = nil
118+
else
119+
@regular_bytes -= task.bytes
120+
end
103121
end
104122
end
105123

‎test/plugin/in_tail/test_worker_pool.rb‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,31 @@ def singleton_override.build_line_feeder; end
9393
pool.acknowledge(first)
9494
pool.acknowledge(second)
9595
assert_equal 0, pool.statistics[:bytes]
96-
assert_false pool.submit(:too_large, 'oversized', bytes: 11)
96+
assert_true pool.submit(:too_large, 'oversized', bytes: 11)
97+
@release << true
98+
pool.acknowledge(result_from(pool))
99+
end
100+
101+
test 'one oversized batch uses a separate reservation beside regular capacity' do
102+
started = Queue.new
103+
pool = create_pool(task_limit: 3, byte_limit: 10, num_threads: 2) do |_, payload|
104+
started << payload
105+
@release.pop
106+
payload
107+
end
108+
assert_true pool.submit(:large, :oversized, bytes: 11)
109+
assert_equal :oversized, Timeout.timeout(5) { started.pop }
110+
assert_true pool.submit(:small, :regular, bytes: 10)
111+
assert_equal :regular, Timeout.timeout(5) { started.pop }
112+
assert_false pool.submit(:another_large, :oversized, bytes: 12)
113+
assert_false pool.submit(:another_small, :regular, bytes: 1)
114+
assert_equal({ tasks: 2, bytes: 21, state: :running }, pool.statistics)
115+
116+
2.times { @release << true }
117+
results = 2.times.map { result_from(pool) }
118+
assert_equal %i[oversized regular], results.map(&:value).sort
119+
results.each { |result| pool.acknowledge(result) }
120+
assert_equal({ tasks: 0, bytes: 0, state: :running }, pool.statistics)
97121
end
98122

99123
test 'a failed task produces an error result and the worker handles the next file' do

‎test/plugin/test_in_tail.rb‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,52 @@ def test_worker_queue_saturation_drains_detached_waiting_file
10311031
assert_empty driver.instance.instance_variable_get(:@worker_deferred_unwatch)
10321032
end
10331033

1034+
def test_oversized_worker_batch_does_not_block_other_files
1035+
tail_input = Fluent::Plugin::TailInput
1036+
original_byte_limit = tail_input.const_get(:WORKER_QUEUE_BYTES)
1037+
tail_input.send(:remove_const, :WORKER_QUEUE_BYTES)
1038+
tail_input.const_set(:WORKER_QUEUE_BYTES, 6)
1039+
large_path = "#{@tmp_dir}/tail-large.txt"
1040+
short_path = "#{@tmp_dir}/tail-short.txt"
1041+
[large_path, short_path].each { |path| Fluent::FileWrapper.open(path, 'wb') {} }
1042+
config = config_element('ROOT', '', {
1043+
'path' => "#{@tmp_dir}/tail-*.txt",
1044+
'pos_file' => "#{@tmp_dir}/tail.pos",
1045+
'tag' => 't1',
1046+
'read_from_head' => true,
1047+
'format' => '/(?<message>.*)/',
1048+
'num_threads' => 2,
1049+
'read_lines_limit' => 1,
1050+
'enable_stat_watcher' => false,
1051+
})
1052+
driver = create_driver(config, false)
1053+
large_started = Queue.new
1054+
continue_large = Queue.new
1055+
released = false
1056+
1057+
driver.run(expect_records: 2, timeout: 10) do
1058+
pool = driver.instance.instance_variable_get(:@worker_pool)
1059+
wrap_worker_pool_process(pool) do |batch|
1060+
if batch.lines.any? { |line| line.include?('oversized-record') }
1061+
large_started << true
1062+
continue_large.pop
1063+
end
1064+
end
1065+
Fluent::FileWrapper.open(large_path, 'ab') { |file| file.puts('oversized-record') }
1066+
Fluent::FileWrapper.open(short_path, 'ab') { |file| file.puts('x') }
1067+
waiting(5) { sleep 0.01 until large_started.size == 1 }
1068+
waiting(5) { sleep 0.01 until driver.events.any? { |event| event[2]['message'] == 'x' } }
1069+
continue_large << true
1070+
released = true
1071+
end
1072+
1073+
assert_equal %w[oversized-record x], driver.events.map { |event| event[2]['message'] }.sort
1074+
ensure
1075+
continue_large << true if continue_large && !released
1076+
tail_input.send(:remove_const, :WORKER_QUEUE_BYTES) if tail_input.const_defined?(:WORKER_QUEUE_BYTES, false)
1077+
tail_input.const_set(:WORKER_QUEUE_BYTES, original_byte_limit) if original_byte_limit
1078+
end
1079+
10341080
def test_worker_queue_saturation_drains_rotated_file
10351081
paths = %w[tail-a.txt tail-b.txt tail-c.txt tail-d.txt tail-e.txt tail-f.txt].map { |name| "#{@tmp_dir}/#{name}" }
10361082
paths.each { |path| Fluent::FileWrapper.open(path, 'wb') {} }

0 commit comments

Comments
 (0)