Support multi workers assign syntax on <worker> section. Fix #2289
#2292
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Hiroshi Hatake <[email protected]>
|
Any advice and feedback are appreciated. |
repeatedly
reviewed
Feb 18, 2019
| raise ConfigError, "worker id #{target_worker_id} specified by <worker> directive is not allowed. Available worker id is between 0 and #{(Fluent::Engine.system_config.workers - 1)}" | ||
| end | ||
| target_worker_ids = target_worker_id_str.split("-") | ||
| if target_worker_ids.size == 2 |
There was a problem hiding this comment.
Could you add duplication check to avoid following case?
<worker 0-3>
</worker>
<worker 3-7> # should be 4-7
</worker>
There was a problem hiding this comment.
I've added a collisions checker in d900a73.
There was a problem hiding this comment.
And I added more concreate error message commit: 03da2b3.
lib/fluent/root_agent.rb
Outdated
| first_worker_id = target_worker_ids.first.to_i | ||
| last_worker_id = target_worker_ids.last.to_i | ||
| if first_worker_id > last_worker_id | ||
| raise ConfigError, "greater first_worker_id<#{first_worker_id}> than last_worker_id<#{last_worker_id}> specified by <worker> directive is not allowed. Available multi worker assign syntax is <smaller_worker_id>-<greater_worker_id>" |
There was a problem hiding this comment.
I used full-qualified name: 4200740.
lib/fluent/root_agent.rb
Outdated
| first_worker_id.step(last_worker_id, 1) do |worker_id| | ||
| target_worker_id = worker_id.to_i | ||
| if target_worker_id < 0 || target_worker_id > (Fluent::Engine.system_config.workers - 1) | ||
| raise ConfigError, "worker id #{target_worker_id} specified by <worker> directive is not allowed. Available worker id is between 0 and #{(Fluent::Engine.system_config.workers - 1)}" |
lib/fluent/root_agent.rb
Outdated
|
|
||
| e.elements.each do |elem| | ||
| unless ['source', 'match', 'filter', 'label'].include?(elem.name) | ||
| raise ConfigError, "<worker> section cannot have <#{elem.name}> directive" |
lib/fluent/root_agent.rb
Outdated
| else | ||
| target_worker_id = target_worker_id_str.to_i | ||
| if target_worker_id < 0 || target_worker_id > (Fluent::Engine.system_config.workers - 1) | ||
| raise ConfigError, "worker id #{target_worker_id} specified by <worker> directive is not allowed. Available worker id is between 0 and #{(Fluent::Engine.system_config.workers - 1)}" |
lib/fluent/root_agent.rb
Outdated
| raise ConfigError, "<worker> section cannot have <#{elem.name}> directive" | ||
| e.elements.each do |elem| | ||
| unless ['source', 'match', 'filter', 'label'].include?(elem.name) | ||
| raise ConfigError, "<worker> section cannot have <#{elem.name}> directive" |
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
cosmo0920
force-pushed
the
support-multi-workers-assign-syntax
branch
from
d900a73 to
6b6fb93
Compare
Signed-off-by: Hiroshi Hatake <[email protected]>
cosmo0920
force-pushed
the
support-multi-workers-assign-syntax
branch
from
297cabb to
03da2b3
Compare
|
This patch something wrong. The result should show two worker ids for each input plugin but it always shows fixed number, e.g. |
|
Oh, |
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
cosmo0920
force-pushed
the
support-multi-workers-assign-syntax
branch
from
c7e1885 to
9b898c1
Compare
|
I've confirmed that the above configuration works correctly with Fluentd LogBoot LogRunning LogAnother Terminal Log |
repeatedly
reviewed
Feb 21, 2019
lib/fluent/root_agent.rb
Outdated
| @@ -64,26 +64,67 @@ def initialize(log:, system_config: SystemConfig.new) | |||
| attr_reader :labels | |||
|
|
|||
| def configure(conf) | |||
| used_worker_ids = [] | |||
| available_worker_ids = [] | |||
| 0.step(Fluent::Engine.system_config.workers - 1, 1).each do |id| | |||
There was a problem hiding this comment.
Can't we use available_worker_ids = (0..Fluent::Engine.system_config.workers - 1).to_a like code?
There was a problem hiding this comment.
Oh.... We can use it. 44f206a
lib/fluent/plugin/base.rb
Outdated
| @@ -52,7 +52,8 @@ def fluentd_worker_id | |||
|
|
|||
| def configure(conf) | |||
| if conf.respond_to?(:for_this_worker?) && conf.for_this_worker? | |||
| system_config_override(workers: 1) | |||
| workers = conf.target_worker_ids.size || 1 | |||
There was a problem hiding this comment.
When conf.target_worker_ids.size return nil or false?
There was a problem hiding this comment.
I've added a checking code on 44b67c9.
Signed-off-by: Hiroshi Hatake <[email protected]>
Signed-off-by: Hiroshi Hatake <[email protected]>
|
@cosmo0920 On your environment, does |
|
In my environment, sometimes IOError error="stream closed in another thread" occurred during shutdown. |
|
I've tested the above configuration with the following code: require 'socket'
u = UDPSocket.new
u.connect('localhost', 5160)
threads = []
threads << Thread.new {
loop do
u.send "uuuu", 0
sleep 0.5
end
}
threads << Thread.new {
loop do
u.send "aaaa", 0
sleep 0.3
end
}
threads << Thread.new {
loop do
u.send "waaa", 0
sleep 0.1
end
}
threads.each { |thr| thr.join }Nothing |
Yeah. Maybe, multiprocess or others trigger it. |
|
Basic behabiour works as expected so merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains a naive
<worker N-M>syntax implementation.Signed-off-by: Hiroshi Hatake [email protected]