Skip to content
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

[fix][offload] Fix numerical overflow bug while reading data from tiered storage #18595

Merged
merged 1 commit into from
Nov 28, 2022

Conversation

dragonls
Copy link
Contributor

@dragonls dragonls commented Nov 24, 2022

Fixes #18580

Motivation

Fix numerical overflow bug while reading data from tiered storage.

Modifications

Fix the BlobStoreBackedInputStreamImpl.available.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is hard to add test case because it might cause OOM.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository: dragonls#5

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 24, 2022
@dragonls
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@dragonls dragonls closed this Nov 25, 2022
@dragonls dragonls reopened this Nov 25, 2022
@dragonls
Copy link
Contributor Author

The test failed but seems not relate to this pr.

@Technoboy- Technoboy- closed this Nov 25, 2022
@Technoboy- Technoboy- reopened this Nov 25, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2022

Codecov Report

Merging #18595 (524a54e) into master (cd85a67) will decrease coverage by 19.09%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #18595       +/-   ##
=============================================
- Coverage     47.39%   28.29%   -19.10%     
+ Complexity    10479     5366     -5113     
=============================================
  Files           698      618       -80     
  Lines         68070    58577     -9493     
  Branches       7279     6094     -1185     
=============================================
- Hits          32264    16577    -15687     
- Misses        32228    39656     +7428     
+ Partials       3578     2344     -1234     
Flag Coverage Δ
unittests 28.29% <ø> (-19.10%) ⬇️

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

Impacted Files Coverage Δ
...n/java/org/apache/pulsar/client/api/RawReader.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pulsar/broker/admin/v1/Brokers.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pulsar/broker/admin/v2/Brokers.java 0.00% <0.00%> (-100.00%) ⬇️
...va/org/apache/pulsar/broker/admin/v1/Clusters.java 0.00% <0.00%> (-100.00%) ⬇️
.../org/apache/pulsar/broker/admin/v1/Properties.java 0.00% <0.00%> (-100.00%) ⬇️
.../org/apache/pulsar/client/impl/RawMessageImpl.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pulsar/broker/admin/v2/ResourceGroups.java 0.00% <0.00%> (-100.00%) ⬇️
...ar/common/naming/PartitionedManagedLedgerInfo.java 0.00% <0.00%> (-100.00%) ⬇️
...naming/RangeEquallyDivideBundleSplitAlgorithm.java 0.00% <0.00%> (-100.00%) ⬇️
...e/pulsar/broker/admin/impl/ResourceQuotasBase.java 0.00% <0.00%> (-96.43%) ⬇️
... and 251 more

@lhotari lhotari merged commit 5c34e84 into apache:master Nov 28, 2022
lhotari pushed a commit that referenced this pull request Nov 28, 2022
…red storage (#18595)

Co-authored-by: druidliu <[email protected]>
(cherry picked from commit 5c34e84)
lhotari pushed a commit that referenced this pull request Nov 28, 2022
…red storage (#18595)

Co-authored-by: druidliu <[email protected]>
(cherry picked from commit 5c34e84)
lhotari pushed a commit that referenced this pull request Nov 28, 2022
…red storage (#18595)

Co-authored-by: druidliu <[email protected]>
(cherry picked from commit 5c34e84)
lhotari pushed a commit that referenced this pull request Nov 28, 2022
…red storage (#18595)

Co-authored-by: druidliu <[email protected]>
(cherry picked from commit 5c34e84)
@lhotari lhotari added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Nov 28, 2022
lhotari pushed a commit to datastax/pulsar that referenced this pull request Nov 28, 2022
…red storage (apache#18595)

Co-authored-by: druidliu <[email protected]>
(cherry picked from commit 5c34e84)
@hangc0276
Copy link
Contributor

Nice catch!

lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Dec 9, 2022
@dragonls dragonls deleted the fix-overflow branch December 21, 2022 03:16
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Reading data from tiered storage might have numerical overflow issue
7 participants