Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[fix][broker] Fix BufferOverflowException and EOFException bugs in /metrics gzip compression #22576

Merged
merged 9 commits into from
Apr 25, 2024

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Apr 24, 2024

Fixes #22575

Motivation

See #22575 . This is a bug introduced by #22521. While fixing the bug, another related bug was revealed where it resulted in a EOFException when reading the gzipped content.

While working on this PR, another unrelated issue in the metrics output buffer pre-allocation was detected. It is also fixed in this PR. The impact of this is about 2x direct memory allocation since the pre-allocated buffers weren't used at all.

Modifications

  • Properly handle boundary conditions where the compression buffer is full.
    • Properly handle the compression state and usage of java.util.zip.Deflater
  • Properly handle writing of the gzip trailer
  • Fix unrelated issue in CompositeBuffer pre-allocation. Use the capacity method to pre-allocate the buffer.
    • The problem was that .setIndex(0, chunkSize) should have been called for the chunk before adding it. This is how the capacity method increases the capacity.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added this to the 3.3.0 milestone Apr 24, 2024
@lhotari lhotari self-assigned this Apr 24, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 24, 2024
@lhotari lhotari requested review from dao-jun and merlimat April 24, 2024 14:38
@lhotari lhotari requested a review from heesung-sn April 24, 2024 19:56
@lhotari lhotari marked this pull request as draft April 25, 2024 05:11
@lhotari lhotari changed the title [fix][broker] Fix BufferOverflowException bug in /metrics gzip compression [fix][broker] Fix BufferOverflowException and EOFException bugs in /metrics gzip compression Apr 25, 2024
@lhotari lhotari marked this pull request as ready for review April 25, 2024 06:51
@lhotari lhotari requested a review from Technoboy- April 25, 2024 07:58
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 79.16667% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 73.95%. Comparing base (bbc6224) to head (4bd5888).
Report is 188 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #22576      +/-   ##
============================================
+ Coverage     73.57%   73.95%   +0.37%     
+ Complexity    32624    32595      -29     
============================================
  Files          1877     1885       +8     
  Lines        139502   140520    +1018     
  Branches      15299    15452     +153     
============================================
+ Hits         102638   103919    +1281     
+ Misses        28908    28565     -343     
- Partials       7956     8036      +80     
Flag Coverage Δ
inttests 26.71% <0.00%> (+2.13%) ⬆️
systests 24.46% <0.00%> (+0.14%) ⬆️
unittests 73.22% <79.16%> (+0.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...r/stats/prometheus/PrometheusMetricsGenerator.java 78.43% <79.16%> (-7.08%) ⬇️

... and 255 files with indirect coverage changes

@lhotari lhotari merged commit 997c8b9 into apache:master Apr 25, 2024
57 of 58 checks passed
lhotari added a commit that referenced this pull request Apr 25, 2024
…etrics gzip compression (#22576)

(cherry picked from commit 997c8b9)
lhotari added a commit that referenced this pull request Apr 25, 2024
…etrics gzip compression (#22576)

(cherry picked from commit 997c8b9)
lhotari added a commit that referenced this pull request Apr 25, 2024
…etrics gzip compression (#22576)

(cherry picked from commit 997c8b9)
lhotari added a commit that referenced this pull request Apr 25, 2024
…etrics gzip compression (#22576)

(cherry picked from commit 997c8b9)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request May 13, 2024
…etrics gzip compression (apache#22576)

(cherry picked from commit 997c8b9)
(cherry picked from commit fe05e08)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request May 16, 2024
…etrics gzip compression (apache#22576)

(cherry picked from commit 997c8b9)
(cherry picked from commit fe05e08)
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] /metrics gzip compression sporadically fails with error 500 caused by java.nio.BufferOverflowException
5 participants