[fix](logstash) Replace HttpClient5 async with HttpClient4 sync to fix CircularRedirectException (1.2.0 -> 1.2.1)#63181
Merged
Conversation
…x CircularRedirectException (1.2.0 -> 1.2.1)
The logstash-output-doris plugin uses Apache HttpClient5 async client to PUT
stream load requests. Against SelectDB Cloud / BYOC FE — which returns
'307 + Connection: close' on stream load — the async client fails with
CircularRedirectException under any meaningful concurrency / body size.
Root cause:
1. HC5 async does not strictly block body transmission while waiting for
'100 Continue'. When FE returns 307 before issuing 100, the entity
producer has already started writing; FE closing the connection then
yields an IOException mid-transfer.
2. HC5 default exec chain wraps RedirectExec around
AsyncHttpRequestRetryExec. The recoverable IOException triggers an
internal retry that re-enters the same FE -> 307 path, but
RedirectLocations from the first attempt is still populated, so the
same BE URL is detected as 'already visited' and reported as a
circular redirect.
This is a real HC5-vs-HC4 implementation difference, not a configuration
issue. The Doris Flink connector also follows FE 307 to BE in its default
path (autoRedirect=true) and works correctly precisely because it uses
HC4 sync: HC4 honors 'Expect: 100-continue' strictly, so when FE 307s
without sending 100, the entity is left unconsumed and HC4's RedirectExec
follows the redirect normally.
This patch aligns the plugin with the Flink connector's HTTP layer:
- bump gem version 1.2.0 -> 1.2.1
- httpclient5 5.4.2 (async) -> httpclient 4.5.13 (sync)
- SimpleRequestBuilder -> HttpPut + ByteArrayEntity (repeatable)
- HttpAsyncClients defaults -> HttpClients with:
* setRequestExecutor(HttpRequestExecutor(60s))
* setRedirectStrategy(DorisRedirectStrategy) (isRedirectable=true,
strip userinfo, normalize empty query)
* setRetryHandler(DefaultHttpRequestRetryHandler(0, false))
* setConnectionReuseStrategy(NoConnectionReuseStrategy.INSTANCE)
* RequestConfig.setExpectContinueEnabled(true)
- Async future plumbing in TableEvents replaced with sync
response_code / response_body / response_error fields.
- Stringify both key and value at request.addHeader call site: HC4's
addHeader(String, String) is strict on types whereas HC5 had a
permissive (String, Object) overload; user configs commonly carry
Float / Integer values like 'max_filter_ratio => 1.0'.
- Drop 's.requirements << jar ...' from gemspec: with JARs vendored under
lib/, the maven lookup at install time is unnecessary and forced users
to set JARS_SKIP=true for offline installs.
Pipeline configuration, retry queue, save_on_failure, group_commit,
label generation, header handling - all unchanged.
Verified on a SelectDB BYOC cluster mirroring the reported production
shape (16 workers x 10000 batch x 200,000 events):
- Before: 100% requests fail with CircularRedirectException
- After: 20/20 stream loads Status=Success, 200,000/200,000 rows
ingested, 0 HTTP-layer errors.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
Author
|
run buildall |
JNSimba
previously approved these changes
May 25, 2026
Member
|
run buildall |
Contributor
Author
|
run buildall |
Member
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I found one blocking packaging regression. The documented build command succeeds, but the resulting gem no longer contains the generated jar loader or HttpClient jars while doris.rb still requires the loader unconditionally.
Critical checkpoint conclusions:
- Goal: switching the Logstash plugin to HttpClient4-style stream-load handling is only partially achieved because the built plugin cannot load from the documented package output.
- Scope: the runtime code change is focused, but the dependency metadata change removes the only visible source for the required jar loader/artifacts without adding an equivalent vendoring/build step.
- Concurrency: the plugin remains
concurrency :shared; the newCloseableHttpClientis designed for concurrent use. No new shared mutable per-request state issue found. - Lifecycle: no new static/global lifecycle issue found, but the plugin load lifecycle fails before registration when the required jar loader is absent.
- Configuration: no new configuration items.
- Compatibility: package/install compatibility regresses for users building/installing from this gemspec because required Java dependencies are no longer resolved or packaged.
- Parallel code paths: only this Logstash extension path is changed.
- Conditional checks: redirect customization is consistent with similar Doris stream-load clients.
- Tests: no test or packaging validation was added; I verified locally that
gem build logstash-output-doris.gemspeccreates a gem whoselib/contains onlylogstash/, notlogstash-output-doris_jars.rbor vendored jars. - Observability: no new observability need for this change.
- Transactions/data writes: stream-load semantics are involved, but I did not find a new transaction correctness issue in the reviewed runtime flow.
- FE/BE variables: not applicable.
- Performance: the new sync client serializes per-table requests that were previously submitted asynchronously, but I am not raising this as a blocker without a concrete workload regression requirement.
User focus: no additional user-provided review focus was supplied.
Reviewer pointed out that the documented `gem build logstash-output-doris.gemspec`
on a clean clone produces a gem whose `lib/` contains only `logstash/` -- no
`logstash-output-doris_jars.rb` loader and no vendored httpclient jars -- so
`require '.../logstash-output-doris_jars.rb'` in `doris.rb` fails at plugin
load time. The jar artifacts and loader are gitignored, so they must be
generated as part of the build.
Restore the dependency declaration so `Jars::Installer.new.vendor_jars` has
a manifest to resolve, and document the full flow in README:
1. vendor_jars -> downloads httpclient4 (+ transitive httpcore /
commons-logging / commons-codec) into lib/ and
generates the _jars.rb loader.
2. gem build -> packages those files into the 1.36 MB
logstash-output-doris-1.2.1-java.gem.
3. install -> `JARS_SKIP=true bin/logstash-plugin install <gem>`
(jars are already vendored, no Maven lookup needed).
4. offline pack -> for air-gapped Logstash hosts, build an offline
pack with `bin/logstash-plugin prepare-offline-pack`
and install via `file://...zip` on the target.
Verified on Logstash 7.17.18 against local Doris docker (FE 8030 / BE 8040):
- clean clone -> vendor_jars -> gem build -> install -> 5/5 stream loads
Status=Success, 5/5 rows in Doris.
- same flow via offline pack on a second Logstash install: 5/5 Success.
Contributor
Author
|
run buildall |
Member
|
run buildall |
Contributor
Author
|
/review |
1 similar comment
Member
|
/review |
Contributor
There was a problem hiding this comment.
No new blocking issues found in this review.
Critical checkpoint conclusions:
- Goal and proof: The PR targets the Logstash stream-load redirect failure by switching from HttpClient5 async to HttpClient4 sync with PUT redirects, disabled HTTP retries, no connection reuse, and Expect: 100-continue. The README now documents the vendoring/build/install flow. The commits describe manual Logstash/Doris verification, but I did not run those tests in this runner.
- Scope: The change is focused on the Logstash extension code, gem dependency metadata, and README build/install documentation.
- Concurrency: The plugin remains
concurrency :shared; the sharedCloseableHttpClientis the intended thread-safe HttpClient usage. The per-batch requests are now synchronous, which is a performance/behavior tradeoff from the old async submission, but I did not identify a correctness issue from it. - Lifecycle/static state: No new cross-TU/static initialization or non-obvious persisted lifecycle issue was found. Existing background thread/client lifecycle patterns are unchanged in kind.
- Configuration: No Doris/Logstash configuration item was added.
- Compatibility/storage format: No storage format, FE/BE protocol, or function symbol compatibility issue was introduced.
- Parallel paths: Related Doris HttpClient stream-load code uses the same redirectable PUT pattern; no missing parallel Doris code path was found for this Logstash-only plugin fix.
- Conditional checks: The new redirect and empty-query normalization checks have clear local rationale.
- Tests/results: No new automated test files or generated outputs are included. Manual verification is described in the commit message; I did not independently execute JRuby/Logstash tests because JRuby is not available in this runner.
- Observability: Existing request/response logging remains in place; no additional observability gap specific to this fix was found.
- Transaction/data correctness: Stream-load success handling and retry/drop/save-on-failure flow are preserved; no new committed/uncommitted data visibility issue was found.
- FE/BE variable passing: Not applicable.
- Performance: The switch from async per-table submission to sync execution can reduce per-batch parallelism when one Logstash batch fans out to many target tables, but this appears inherent to the selected sync HttpClient4 approach and not a blocking correctness regression.
Existing review context: the earlier inline thread about missing vendored jar generation should not be duplicated; the current head restores s.requirements for HttpClient4 and documents vendor_jars before gem build.
User focus: no additional user-provided review focus was specified.
This was referenced Jul 17, 2026
Closed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Replace HttpClient5 async with HttpClient4 sync to fix CircularRedirectException (1.2.0 -> 1.2.1)
The logstash-output-doris plugin uses Apache HttpClient5 async client to PUT stream load requests. Against SelectDB Cloud / BYOC FE — which returns '307 + Connection: close' on stream load — the async client fails with CircularRedirectException under any meaningful concurrency / body size.
Root cause:
This is a real HC5-vs-HC4 implementation difference, not a configuration issue. The Doris Flink connector also follows FE 307 to BE in its default path (autoRedirect=true) and works correctly precisely because it uses HC4 sync: HC4 honors 'Expect: 100-continue' strictly, so when FE 307s without sending 100, the entity is left unconsumed and HC4's RedirectExec follows the redirect normally.
This patch aligns the plugin with the Flink connector's HTTP layer:
strip userinfo, normalize empty query)
response_code / response_body / response_error fields.
addHeader(String, String) is strict on types whereas HC5 had a
permissive (String, Object) overload; user configs commonly carry
Float / Integer values like 'max_filter_ratio => 1.0'.
lib/, the maven lookup at install time is unnecessary and forced users
to set JARS_SKIP=true for offline installs.
Pipeline configuration, retry queue, save_on_failure, group_commit, label generation, header handling - all unchanged.
Verified on a SelectDB BYOC cluster mirroring the reported production shape (16 workers x 10000 batch x 200,000 events):
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)