[Backport 2.x] Use new instance of Decompressor on channel initialization #3598
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
![opensearch-trigger-bot[bot]](https://avatars.githubusercontent.com/u/80134844?s=60&v=4){kind=small}
### Description Resolves an issue with decompression that can lead to concurrent gzipped requests failing. This removes the `@Sharable` annotation from the `Netty4ConditionalDecompressor` and creates a new instance of the decompressor on channel initialization. `Netty4ConditionalDecompressor` is an `HttpContentDecompressor` which is a subclass of `HttpContentDecoder` - a stateful handler. Netty docs on `@Sharable` annotation: https://netty.io/4.0/api/io/netty/channel/ChannelHandler.Sharable.html * Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation) Bug fix ### Issues Resolved - opensearch-project/OpenSearch#10802 ### Testing Tested by running OpenSearch w fluentbit and Merge_Log on. See files below which can reproduce the issue from the linked error. I opened this PR as draft pending an integration test to validate the behavior. `docker-compose.yml` ``` version: '3' services: opensearch: # This is also the hostname of the container within the Docker network (i.e. https://opensearch-node1/) image: opensearchproject/opensearch:latest # Specifying the latest available image - modify if you want a specific version container_name: opensearch environment: - cluster.name=opensearch-cluster # Name the cluster - node.name=opensearch # Name the node that will run in this container - discovery.type=single-node - bootstrap.memory_lock=true # Disable JVM heap memory swapping - "OPENSEARCH_JAVA_OPTS=-Xms512m -Xmx512m" # Set min and max JVM heap sizes to at least 50% of system RAM ulimits: memlock: soft: -1 # Set memlock to unlimited (no soft or hard limit) hard: -1 nofile: soft: 65536 # Maximum number of open files for the opensearch user - set to at least 65536 hard: 65536 volumes: - opensearch-data1:/usr/share/opensearch/data # Creates volume called opensearch-data1 and mounts it to the container # - /Users/craigperkins/Projects/OpenSearch/security/build/distributions/opensearch-security-2.11.0.0-SNAPSHOT.jar:/usr/share/opensearch/plugins/opensearch-security/opensearch-security-2.11.0.0.jar ports: - 9200:9200 # REST API - 9600:9600 # Performance Analyzer networks: - opensearch-net # All of the containers will join the same Docker bridge network fluent-bit: image: fluent/fluent-bit volumes: - ./fluent-bit.conf:/fluent-bit/etc/fluent-bit.conf depends_on: - opensearch networks: - opensearch-net volumes: opensearch-data1: opensearch-data2: networks: opensearch-net: ``` `fluent-bit.conf` ``` [INPUT] Name dummy Dummy {"top": {".dotted": "value"}} [OUTPUT] Name es Host opensearch Port 9200 HTTP_User admin HTTP_Passwd admin Replace_Dots On Suppress_Type_Name On Compress gzip tls On tls.verify Off net.keepalive Off [FILTER] Name kubernetes Match kube.* Buffer_Size 256KB Merge_Log On Keep_Log On ``` ### Check List - [ ] New functionality includes testing - [ ] New functionality has been documented - [x] Commits are signed per the DCO using --signoff By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). --------- Signed-off-by: Craig Perkins <craig5008@gmail.com> Signed-off-by: Craig Perkins <cwperx@amazon.com> Signed-off-by: Peter Nied <petern@amazon.com> Co-authored-by: Peter Nied <petern@amazon.com> (cherry picked from commit 499db78) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot
bot
requested review from
cliu123,
cwperks,
DarshitChanpura,
davidlago,
peternied,
RyanL1997,
scrawfor99,
reta and
willyborankin
as code owners
Codecov Report
@@ Coverage Diff @@
## 2.x #3598 +/- ##
============================================
+ Coverage 64.74% 64.79% +0.05%
- Complexity 3589 3591 +2
============================================
Files 282 282
Lines 20381 20379 -2
Branches 3372 3372
============================================
+ Hits 13196 13205 +9
+ Misses 5510 5501 -9
+ Partials 1675 1673 -2
|
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
|
Thank you @DarshitChanpura ! |
cwperks
approved these changes
Oct 25, 2023
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.
Backport 499db78 from #3583.