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.

Sign up for GitHub

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

Jump to bottom

[fix][broker] Fix invalid condition in logging exceptions #22412

Merged
merged 1 commit into from
Apr 3, 2024
Merged

[fix][broker] Fix invalid condition in logging exceptions #22412

merged 1 commit into from
Apr 3, 2024

Conversation

teet-vaher-sympower
Copy link
Contributor

@teet-vaher-sympower teet-vaher-sympower commented Apr 3, 2024 •
edited by lhotari
Loading

Fixes #22409

Motivation

If Admin API request is sent to the broker and it responds with 307-redirect (for example in case of the bundle is not served by this broker) then ERROR is written into the broker log file. And there are quite a lot of these errors.

> GET https://current-pulsar-server:8443/admin/v2/non-persistent/tenant/namespace/0x00000000_0x40000000

< HTTP/1.1 307 Temporary Redirect
< Date: Tue, 02 Apr 2024 11:19:32 GMT
< Location: https://another-pulsar-server:8443/admin/v2/non-persistent/tenant/namespace/0x00000000_0x40000000?authoritative=false
< Content-Length: 0

broker log:

11:18:57.455 [pulsar-web-36-6] ERROR org.apache.pulsar.broker.admin.v2.NonPersistentTopics - [superuser] Failed to list topics on namespace bundle tenant/namespace/0x00000000_0x40000000
java.util.concurrent.CompletionException: javax.ws.rs.WebApplicationException: HTTP 307 Temporary Redirect
        at java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315) ~[?:?]
...

Modifications

Issue #21995 introduced a new variable isNot307And404Exception. Previous code was substituted with the new variable in multiple places, but the exclamation mark in front of the variable (part of the previous code) was not removed. This PR removes the remaining unnecessary exclamation marks.

Verifying this change

  • Make sure that the change passes the CI checks.

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

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
  • The metrics
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository: teet-vaher-sympower#1

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 3, 2024

@teet-vaher-sympower Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@lhotari lhotari added this to the 3.3.0 milestone Apr 3, 2024
@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Apr 3, 2024
@lhotari lhotari changed the title [fix][broker] Remove faulty exclamation marks [fix][broker] Fix invalid condition in filtering exception logging Apr 3, 2024
@lhotari lhotari changed the title [fix][broker] Fix invalid condition in filtering exception logging [fix][broker] Fix invalid condition in logging exceptions Apr 3, 2024
@lhotari
Copy link
Member

lhotari commented Apr 3, 2024 •
edited
Loading

Great discovery. Thanks for the contribution @teet-vaher-sympower !

lhotari
lhotari approved these changes Apr 3, 2024
View reviewed changes
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

dao-jun
dao-jun approved these changes Apr 3, 2024
View reviewed changes
Copy link
Member

@dao-jun dao-jun left a comment

Choose a reason for hiding this comment

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

lgtm

@lhotari lhotari merged commit cd6f53b into apache:master Apr 3, 2024
63 of 69 checks passed
lhotari pushed a commit that referenced this pull request Apr 3, 2024
@teet-vaher-sympower @lhotari
[fix][broker] Fix invalid condition in logging exceptions (#22412)
a49e8a6 
(cherry picked from commit cd6f53b)
lhotari pushed a commit that referenced this pull request Apr 3, 2024
@teet-vaher-sympower @lhotari
[fix][broker] Fix invalid condition in logging exceptions (#22412)
afe4e18 
(cherry picked from commit cd6f53b)
lhotari pushed a commit that referenced this pull request Apr 3, 2024
@teet-vaher-sympower @lhotari
[fix][broker] Fix invalid condition in logging exceptions (#22412)
71ce55b 
(cherry picked from commit cd6f53b)
lhotari pushed a commit that referenced this pull request Apr 3, 2024
@teet-vaher-sympower @lhotari
[fix][broker] Fix invalid condition in logging exceptions (#22412)
5dac342 
(cherry picked from commit cd6f53b)
lhotari pushed a commit that referenced this pull request Apr 3, 2024
@teet-vaher-sympower @lhotari
[fix][broker] Fix invalid condition in logging exceptions (#22412)
265e4ed 
(cherry picked from commit cd6f53b)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 3, 2024
@teet-vaher-sympower @nikhil-ctds
[fix][broker] Fix invalid condition in logging exceptions (apache#22412)
86d3fe8 
(cherry picked from commit cd6f53b)
(cherry picked from commit 265e4ed)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 4, 2024
@teet-vaher-sympower @nikhil-ctds
[fix][broker] Fix invalid condition in logging exceptions (apache#22412)
cca9630 
(cherry picked from commit cd6f53b)
(cherry picked from commit 265e4ed)
nodece pushed a commit to ascentstream/pulsar that referenced this pull request May 13, 2024
@teet-vaher-sympower @nodece
[fix][broker] Fix invalid condition in logging exceptions (apache#22412)
3588062 
(cherry picked from commit cd6f53b)
@teet-vaher-sympower teet-vaher-sympower deleted the fix/remove_faulty_excl_marks branch May 21, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poorbarcode poorbarcode poorbarcode approved these changes

@dao-jun dao-jun dao-jun approved these changes

@lhotari lhotari lhotari approved these changes

@Technoboy- Technoboy- Awaiting requested review from Technoboy-

Assignees

@teet-vaher-sympower teet-vaher-sympower

Labels
cherry-picked/branch-2.10 cherry-picked/branch-2.11 cherry-picked/branch-3.0 cherry-picked/branch-3.1 cherry-picked/branch-3.2 doc-not-needed Your PR changes do not impact docs ready-to-test release/2.10.7 release/2.11.5 release/3.0.5 release/3.1.4 release/3.2.3 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

[Bug][broker] Broker contains invalid condition for logging exceptions
4 participants
@teet-vaher-sympower @lhotari @dao-jun @poorbarcode