support DNS SRV record by RFC2782 #2496
Conversation
bootjp
force-pushed
the
master
branch
2 times, most recently
from
adb4c61 to
c0231ca
Compare
Signed-off-by: ぶーと / Yoshiaki Ueda <[email protected]>
Signed-off-by: ぶーと / Yoshiaki Ueda <[email protected]>
|
@ganmacs How about this? You are now working on pluggable SD on out_forward. |
Right. but anyway, I'll take a look this later. |
|
|
||
| # @param host string | ||
| # @return [SRV] | ||
| def resolve_srv(host) |
There was a problem hiding this comment.
SRV records should be cached for performance.
And specifying the timing of purging cache for users is better.
| @@ -620,6 +620,69 @@ def read_ack_from_sock(sock, unpacker) | |||
| assert{ logs.any?{|log| log.include?("no response from node. regard it as unavailable.") } } | |||
| end | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Can you add a test which checks if the SRV record service discovery works?
There was a problem hiding this comment.
Yes, should this use a real domain name?
There was a problem hiding this comment.
I don't think so. mocking or stubbing seems enough.
Signed-off-by: bootjp <[email protected]>
Signed-off-by: bootjp <[email protected]>
There was a problem hiding this comment.
I think updating caches should be invoked in another thread since performance issue.
Creating EvenTime and comparing it every write isn't efficient. And Node#resolve_dns is called by multiple threads, right? This means that if there is the only Node which uses this feature and multiple threads try to write (and the cache is expired), every thread tries to resolve the SRV record.
| end | ||
|
|
||
| # swap config host to srv response host. | ||
| @srv_host_mutex.synchronize do |
There was a problem hiding this comment.
If thread1(t1) and thread2(t2) which use the same Node try to write, thread1 and thread2 can use the same target.
Is it intentional?
- t1 does L918(and get target1)
- t2 does L918(and get target2)
- t1 does L930~L938,(and set
@hostand@porttarget1) - context switch from t1 to t2 (t1 is still L939)
- t2 does L930~L938,(and set
@hostand@porttarget2) - t1 and t2 send message to target2
There was a problem hiding this comment.
We need to lock other places where node uses @host and @port(e.g. https://github.com/fluent/fluentd/pull/2496/files#diff-40c0e8c88efdc9a297a9fc63bc2bc69fR948).
This code can be a race condition.
- t1 tries to do this line and evaluates arguments
@host - context switch happens to t2
- t2 does L930~L938,(and set
@hostand@portto another value) - context switch happens to t1
@portis invalid for evaluating@hostat step 1 since@porthas changed by t2
|
Close because it seems to be implemented in #2541 . |
|
We are now considering DNS SRV record based SD plugin in the core. |
Which issue(s) this PR fixes:
Fixes : None
What this PR does / why we need it:
https://groups.google.com/forum/#!topic/fluentd/Lxx0-rVtsfo
Support for RFC2782 in out_forward allows for more flexible fluentd operations
Specifically, public cloud service discovery
Docs Changes:
It will be necessary when this request is merged.
I will send a pull request to the document separately.
Release Note:
To the reviewer
SRV#available? may be a better implementation.
For example,
fluent/compat/socket_utilmight seem like a smarter way to implement it, but fluentd's code is too complex for my technical capabilities to figure out how to do it.I will correct the code better if you give me advice.