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

[improve][meta] Improve fault tolerance of blocking calls by supporting timeout #21028

Merged
merged 10 commits into from Aug 21, 2023
Merged

[improve][meta] Improve fault tolerance of blocking calls by supporting timeout #21028

merged 10 commits into from Aug 21, 2023

Conversation

mattisonchao
Copy link
Member

Motivation

We've got a lot of blocking calls without a timeout in the BookKeeper metadata plugin. In a bad situation, it will trigger some deadlocks.

Modifications

  • Improve the fault tolerance of blocking calls by supporting timeout.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

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

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 18, 2023
@mattisonchao mattisonchao self-assigned this Aug 18, 2023
@mattisonchao mattisonchao added this to the 3.2.0 milestone Aug 18, 2023
@hangc0276
Copy link
Contributor

@horizonzy Please help take a look, thanks.

@mattisonchao mattisonchao reopened this Aug 19, 2023
@mattisonchao mattisonchao changed the title [meta] Improve fault tolerance of blocking calls by supporting timeout [improve][meta] Improve fault tolerance of blocking calls by supporting timeout Aug 19, 2023
Copy link
Member

@horizonzy horizonzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here should also be catch

@codecov-commenter
Copy link

Codecov Report

Merging #21028 (c0df801) into master (0cb1c78) will increase coverage by 39.58%.
Report is 6 commits behind head on master.
The diff coverage is 62.42%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #21028       +/-   ##
=============================================
+ Coverage     33.56%   73.14%   +39.58%     
- Complexity    12198    32267    +20069     
=============================================
  Files          1621     1875      +254     
  Lines        126970   139577    +12607     
  Branches      13857    15344     +1487     
=============================================
+ Hits          42618   102095    +59477     
+ Misses        78748    29403    -49345     
- Partials       5604     8079     +2475     
Flag Coverage Δ
inttests 24.23% <1.73%> (-0.01%) ⬇️
systests 25.25% <0.57%> (?)
unittests 72.41% <62.42%> (+40.31%) ⬆️

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

Files Changed Coverage Δ
...ookkeeper/PulsarLedgerUnderreplicationManager.java 51.09% <52.85%> (+51.09%) ⬆️
...etadata/bookkeeper/PulsarLedgerManagerFactory.java 61.53% <57.14%> (+30.98%) ⬆️
...ookkeeper/LongHierarchicalLedgerRangeIterator.java 81.66% <66.66%> (+81.66%) ⬆️
...metadata/bookkeeper/PulsarRegistrationManager.java 62.56% <70.96%> (+53.16%) ⬆️
...kkeeper/LegacyHierarchicalLedgerRangeIterator.java 72.15% <71.42%> (+72.15%) ⬆️
...ulsar/metadata/bookkeeper/PulsarLayoutManager.java 66.66% <77.77%> (+46.66%) ⬆️
...ar/metadata/bookkeeper/AbstractMetadataDriver.java 75.92% <100.00%> (+26.86%) ⬆️

... and 1514 files with indirect coverage changes

Copy link
Member

@horizonzy horizonzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Technoboy- Technoboy- merged commit 976a580 into apache:master Aug 21, 2023
45 checks passed
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.

None yet

7 participants