-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[fix](logstash) Add HTTP timeouts and SO_KEEPALIVE to prevent pipeline hang #65766
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,7 @@ class LogStash::Outputs::Doris < LogStash::Outputs::Base | |
| java_import 'org.apache.http.impl.NoConnectionReuseStrategy' | ||
| java_import 'org.apache.http.protocol.HttpRequestExecutor' | ||
| java_import 'org.apache.http.client.config.RequestConfig' | ||
| java_import 'org.apache.http.config.SocketConfig' | ||
|
|
||
| # support multi thread concurrency for performance | ||
| # so multi_receive() and function it calls are all stateless and thread safe | ||
|
|
@@ -88,6 +89,17 @@ class LogStash::Outputs::Doris < LogStash::Outputs::Base | |
| # max retry queue size in MB, default is 20% max memory of JVM | ||
| config :max_retry_queue_mb, :validate => :number, :default => java.lang.Runtime.get_runtime.max_memory / 1024 / 1024 / 5 | ||
|
|
||
| # HTTP connect timeout in milliseconds (time to establish TCP connection) | ||
| config :connect_timeout_ms, :validate => :number, :default => 60_000 | ||
|
|
||
| # HTTP connection request timeout in milliseconds (time to lease a connection from the pool) | ||
| config :connection_request_timeout_ms, :validate => :number, :default => 60_000 | ||
|
|
||
| # HTTP socket / read timeout in milliseconds (time waiting for response data). | ||
| # Without this, a TCP half-open connection can block the worker thread forever. | ||
| # Default 600s to leave headroom for large stream loads. | ||
| config :socket_timeout_ms, :validate => :number, :default => 600_000 | ||
|
|
||
| def print_plugin_info() | ||
| @plugins = Gem::Specification.find_all{|spec| spec.name =~ /logstash-output-doris/ } | ||
| @plugin_name = @plugins[0].name | ||
|
|
@@ -122,22 +134,39 @@ def http_query(table) | |
| end | ||
|
|
||
| def register | ||
| # HttpClient 4.5.13 sync — same setup as Doris Flink connector (HttpUtil.java) | ||
| # HttpClient 4.5.13 sync — same setup as Doris Flink / Kettle connectors (HttpUtil.java) | ||
| # Key points: | ||
| # - setRequestExecutor(60s) : long wait for 100-continue, FE may delay 307 under load | ||
| # - setRedirectStrategy : follow 307 on PUT (default DefaultRedirectStrategy refuses) | ||
| # - DefaultHttpRequestRetryHandler(0, false) : NO retries -> avoid spurious CircularRedirect | ||
| # - NoConnectionReuseStrategy : one connection per request, dodge keep-alive half-close | ||
| # - setExpectContinueEnabled(true) : critical -> HC4 waits for 100, FE 307s before body is sent, | ||
| # entity stays unconsumed, RedirectExec follows successfully | ||
| # - RequestConfig timeouts : prevent worker threads from hanging forever on TCP | ||
| # half-open connections (connect / pool / socket read) | ||
| # - SocketConfig SO_KEEPALIVE : let the kernel probe dead peers even without reuse | ||
| request_config = RequestConfig.custom | ||
| .setConnectTimeout(@connect_timeout_ms) | ||
| .setConnectionRequestTimeout(@connection_request_timeout_ms) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please size the connection pool before making pool leases expire after 60 seconds. |
||
| .setSocketTimeout(@socket_timeout_ms) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A timeout-driven retry is not safe merely because it reuses the label. The retried HTTP request gets a new Stream Load request ID; if the original transaction is still |
||
| .setExpectContinueEnabled(true) | ||
| .build | ||
| socket_config = SocketConfig.custom | ||
| .setSoKeepAlive(true) | ||
| .build | ||
| @client = HttpClients.custom | ||
| .setRequestExecutor(HttpRequestExecutor.new(60_000)) | ||
| .setRedirectStrategy(DorisRedirectStrategy.new) | ||
| .setRetryHandler(DefaultHttpRequestRetryHandler.new(0, false)) | ||
| .setConnectionReuseStrategy(NoConnectionReuseStrategy::INSTANCE) | ||
| .setDefaultRequestConfig(RequestConfig.custom.setExpectContinueEnabled(true).build) | ||
| .setDefaultRequestConfig(request_config) | ||
| .setDefaultSocketConfig(socket_config) | ||
| .build | ||
|
|
||
| @logger.info("http timeouts (ms): connect=#{@connect_timeout_ms}, " \ | ||
| "connection_request=#{@connection_request_timeout_ms}, " \ | ||
| "socket=#{@socket_timeout_ms}; so_keepalive=true") | ||
|
|
||
| @request_headers = make_request_headers | ||
| @logger.info("request headers: ", @request_headers) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These three public settings accept a wider domain than the Java API they feed. Logstash's
:numberaccepts fractions, and on both README-supported JRuby lines0.5is truncated to Java0; HttpClient interprets zero as an infinite timeout. Negative values likewise leave the finite-timeout guarantee undefined/disabled, while values above2147483647raiseRangeErrorduringregister. The startup log prints the original Ruby value, so it can even claim0.5ms while the effective timeout is infinite. Please validate positive whole milliseconds in1..2147483647, pass an explicit Javaint, document the range, and add JRuby-facing boundary tests for all three options.