Repository navigation
Conversation
…utStream to reuse the Connection
|
💔 -1 overall
This message was automatically generated. |
jojochuang
reviewed
Oct 13, 2026
jojochuang
left a comment
jojochuang
left a comment
Contributor
There was a problem hiding this comment.
BTW I'm also curious if the problem is dependent on the fact that Ranger KMS is Tomcat based not Jetty based (Hadoop KMS). Just a hunch I want to bring up.
| */ | ||
| public static final String DRAIN_RESPONSE_UNREAD_BYTES = CONFIG_PREFIX | ||
| + "drain.response.unread.bytes"; | ||
| public static final boolean DEFAULT_DRAIN_RESPONSE_UNREAD_BYTES = false; |
Contributor
There was a problem hiding this comment.
let's make it true by default, because we'll rely on this new behavior into the future.
Contributor
Author
@jojochuang , thanks for the review. let me add more details: Hadoop-KMS and Ranger-KMS has same response write path: where it uses StreamEncoder as part of OutputStreamWriter to take care of encoding. This StreamEncoder has behaviour change in Java 8 & Java 17. On server side, Jackson calls the close. On a Java 8 server nothing flushes early, OutputBuffer.close() sets Content-Length and the whole response is one record; the client uses KeepAliveStream, not ChunkedInputStream. As per my analysis, same issue may happen with Hadoop-KMS as well but I have not yet verified this for Hadoop-KMS. |
…re-default. Default hadoop.security.kms.client.drain.response.unread.bytes to true so JDK 17 clients reuse keep-alive connections to chunked KMS responses; add core-default.xml description and opt-out guidance. Co-authored-by: Cursor <cursoragent@cursor.com>
|
💔 -1 overall
This message was automatically generated. |
…lic. Fix TestCommonConfigurationFields by declaring the core-default property alongside other KMS client keys and referencing it from KMSClientProvider. Co-authored-by: Cursor <cursoragent@cursor.com>
Expose isChunkedHttpResponse for testing and cover IOUtils.drainAndClose behavior including the 8 MiB read cap and HttpURLConnection error stream. Co-authored-by: Cursor <cursoragent@cursor.com>
jojochuang
marked this pull request as ready for review
October 13, 2026 22:08
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
This branch has not been deployed
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
3 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.
…utStream to reuse the Connection
Description of PR
One performance issue was observed when migrated from JDK 8 to JDK 17. Considerable throughput fall was observed with Ranger-KMS service. On analysis, like by observing the sockets being created, It was found that KeepAlive cache was not working. TLS debug log (shared in JIRA) suggested that it is not trying to cache the connection while closing the stream.
When response size increases beyond the configured threshold ( In Tomcat, it's 8 KB ), Content-Type is set to "chunked" and ChunkedInputStream is used to read the response. No issues here.
Issue is occurring when complete JSON payload comes in one TLS record and chunk terminator bytes are coming in a separate TLS record. Jackson , that is being used to read the response, calls inputStream.close() as soon as it reads the entire payload, that is closing braces (}) and doesn't wait for the terminator chunk to arrive.
At this point, last chunk containing the terminator bytes has not yet arrived or has not yet been unwrapped, and ChunkedInputStream.close()---> hurry() tries to clean the unread bytes one last time without getting blocked. This operation is non-blocking by default. As a result, JVM was not able to clean the underlying inputStream and it can not be reused, hence connection is being destroyed. As a result, each call to ranger-KMS is doing a fresh new TLS handshaking.
FIX: As a fix, chunk terminator bytes or any unread bytes from response inputStream is being drained explicitly before calling close() on the underlying stream. To prevent Jackson from calling close() automatically just after reading the json payload, JsonParser.Feature.AUTO_CLOSE_SOURCE is being disabled. But please note that close() on underlying stream is being called from the finally block in both cases.
Further, logic to explicitly drain and close is being kept behind a flag which is set to "false" by default. It can be enabled by adding following property in the core-site.xml file :
Cluster patched with this fix improved the throughput around three times and average latency also went down significantly.
How was this patch tested?
Note: I had to skip few unrelated UTs for my local build as it was stuck for very long time, like org.apache.hadoop.security.TestDoAsEffectiveUser
For code changes:
declared according to the connector-specific documentation? Note: Automated CI
testing doesn't cover all cases so manual testing with cloud storage is still
required. ==> N/A
LICENSE,LICENSE-binary,NOTICE-binaryfiles? ==? N/AAI Tooling
If an AI tool was used:
where is the name of the AI tool used. ==> N/A
https://www.apache.org/legal/generative-tooling.html ==> N/A