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

[cleanup] Catch TimeoutException when logging about time outs #20349

Merged
merged 1 commit into from May 18, 2023

Conversation

michaeljmarshall
Copy link
Member

Relates to: #20299

Motivation

We have several catch blocks that are dedicated to providing meaningful logs about timeouts. As such, we should catch the TimeoutException, not the InterruptedException, in order to ensure accurate logs.

Modifications

  • Replace InterruptedException with TimeoutException

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Documentation

  • doc-not-needed

@michaeljmarshall michaeljmarshall added type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use doc-not-needed Your PR changes do not impact docs ready-to-test labels May 18, 2023
@michaeljmarshall michaeljmarshall added this to the 3.1.0 milestone May 18, 2023
@michaeljmarshall michaeljmarshall self-assigned this May 18, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 18, 2023

Codecov Report

Merging #20349 (a9e902e) into master (1a66b64) will increase coverage by 38.37%.
The diff coverage is 30.98%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #20349       +/-   ##
=============================================
+ Coverage     34.55%   72.92%   +38.37%     
- Complexity    12631    31861    +19230     
=============================================
  Files          1614     1864      +250     
  Lines        126234   138313    +12079     
  Branches      13779    15172     +1393     
=============================================
+ Hits          43622   100871    +57249     
+ Misses        76999    29438    -47561     
- Partials       5613     8004     +2391     
Flag Coverage Δ
inttests 24.15% <7.04%> (+0.03%) ⬆️
systests 25.03% <7.04%> (?)
unittests 72.21% <30.98%> (+38.91%) ⬆️

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

Impacted Files Coverage Δ
...hentication/oidc/AuthenticationProviderOpenID.java 75.86% <ø> (+75.86%) ⬆️
...org/apache/pulsar/broker/ServiceConfiguration.java 99.37% <ø> (+1.45%) ⬆️
...sar/broker/authorization/AuthorizationService.java 57.20% <0.00%> (+46.80%) ⬆️
...apache/pulsar/broker/namespace/OwnershipCache.java 85.26% <ø> (+12.63%) ⬆️
...java/org/apache/pulsar/broker/rest/TopicsBase.java 58.45% <0.00%> (+58.45%) ⬆️
...a/org/apache/pulsar/websocket/ConsumerHandler.java 62.50% <0.00%> (+35.91%) ⬆️
...a/org/apache/pulsar/websocket/ProducerHandler.java 63.21% <0.00%> (+30.29%) ⬆️
...ava/org/apache/pulsar/websocket/ReaderHandler.java 47.05% <0.00%> (+47.05%) ⬆️
...ache/pulsar/broker/namespace/NamespaceService.java 69.93% <20.00%> (+26.29%) ⬆️
...pache/pulsar/broker/admin/impl/NamespacesBase.java 71.76% <66.66%> (+45.65%) ⬆️
... and 2 more

... and 1498 files with indirect coverage changes

@michaeljmarshall michaeljmarshall merged commit 65f6112 into apache:master May 18, 2023
43 checks passed
@michaeljmarshall michaeljmarshall deleted the fix-timeout-error branch May 18, 2023 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs ready-to-test type/cleanup Code or doc cleanups e.g. remove the outdated documentation or remove the code no longer in use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants