-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 the null point caused by deleting the system topic policy #12367
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nodece
force-pushed
the
filter_policy_messages
branch
from
October 14, 2021 15:35
da5773b
to
51762f3
Compare
/pulsarbot run-failure-checks |
codelipenghui
requested review from
hangc0276,
codelipenghui,
315157973,
eolivelli and
merlimat
October 15, 2021 03:04
315157973
reviewed
Oct 15, 2021
...ker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java
Outdated
Show resolved
Hide resolved
nodece
force-pushed
the
filter_policy_messages
branch
from
October 15, 2021 04:05
51762f3
to
44ed11f
Compare
nodece
force-pushed
the
filter_policy_messages
branch
from
October 15, 2021 04:21
bc2715b
to
5b78442
Compare
nodece
changed the title
Fix update system topic policy
Fix the null point caused by deleting the system theme policy
Oct 15, 2021
nodece
commented
Oct 15, 2021
...r-broker/src/main/java/org/apache/pulsar/broker/systopic/TopicPoliciesSystemTopicClient.java
Outdated
Show resolved
Hide resolved
315157973
requested changes
Oct 15, 2021
...r-broker/src/main/java/org/apache/pulsar/broker/systopic/TopicPoliciesSystemTopicClient.java
Outdated
Show resolved
Hide resolved
nodece
force-pushed
the
filter_policy_messages
branch
from
October 15, 2021 11:48
5b78442
to
e03d403
Compare
eolivelli
changed the title
Fix the null point caused by deleting the system theme policy
Fix the null point caused by deleting the system policy
Oct 15, 2021
nodece
changed the title
Fix the null point caused by deleting the system policy
Fix the null point caused by deleting the topic policy
Oct 18, 2021
nodece
changed the title
Fix the null point caused by deleting the topic policy
Fix the null point caused by deleting the system topic policy
Oct 18, 2021
315157973
reviewed
Oct 18, 2021
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/TopicPoliciesTest.java
Outdated
Show resolved
Hide resolved
nodece
force-pushed
the
filter_policy_messages
branch
3 times, most recently
from
October 18, 2021 04:09
8467088
to
7dd4d2a
Compare
/pulsarbot run-failure-checks |
nodece
force-pushed
the
filter_policy_messages
branch
from
October 18, 2021 06:48
7dd4d2a
to
0c72bcb
Compare
315157973
reviewed
Oct 18, 2021
pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/TopicPoliciesTest.java
Show resolved
Hide resolved
315157973
reviewed
Oct 18, 2021
...ker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java
Outdated
Show resolved
Hide resolved
nodece
force-pushed
the
filter_policy_messages
branch
2 times, most recently
from
October 18, 2021 07:42
b58ebb9
to
230a91a
Compare
315157973
approved these changes
Oct 18, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
RobertIndie
reviewed
Oct 19, 2021
...ker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Zixuan Liu <[email protected]>
nodece
force-pushed
the
filter_policy_messages
branch
from
October 20, 2021 02:43
230a91a
to
038640b
Compare
RobertIndie
approved these changes
Oct 20, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
zymap
approved these changes
Oct 22, 2021
zymap
pushed a commit
that referenced
this pull request
Oct 22, 2021
Signed-off-by: Zixuan Liu <[email protected]> ### Motivation `Message<PulsarEvent>.getValue()` sometimes returns `null` in `SystemTopicBasedTopicPoliciesService.notifyListener()`, so we need to skip the messages that do not belong to the policy type, this problem can cause the policy service to fail to work. ### Modifications - Checks the `Message<PulsarEvent>.getValue()` value. - Uses the `event` instead of `null` as message value when delete policy. ### Verifying this change - [x] Make sure that the change passes the CI checks. ### Does this pull request potentially affect one of the following parts: *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): no - The public API: no - The schema: no - The default values of configurations: no - The wire protocol: no - The rest endpoints: no - The admin cli options: no - Anything that affects deployment: no ### Documentation Check the box below and label this PR (if you have committer privilege). Need to update docs? - [ ] doc-required - [x] no-need-doc - [ ] doc (cherry picked from commit d310e79)
eolivelli
pushed a commit
to eolivelli/pulsar
that referenced
this pull request
Nov 29, 2021
…#12367) Signed-off-by: Zixuan Liu <[email protected]> ### Motivation `Message<PulsarEvent>.getValue()` sometimes returns `null` in `SystemTopicBasedTopicPoliciesService.notifyListener()`, so we need to skip the messages that do not belong to the policy type, this problem can cause the policy service to fail to work. ### Modifications - Checks the `Message<PulsarEvent>.getValue()` value. - Uses the `event` instead of `null` as message value when delete policy. ### Verifying this change - [x] Make sure that the change passes the CI checks. ### Does this pull request potentially affect one of the following parts: *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): no - The public API: no - The schema: no - The default values of configurations: no - The wire protocol: no - The rest endpoints: no - The admin cli options: no - Anything that affects deployment: no ### Documentation Check the box below and label this PR (if you have committer privilege). Need to update docs? - [ ] doc-required - [x] no-need-doc - [ ] doc
codelipenghui
pushed a commit
that referenced
this pull request
Jan 10, 2022
Signed-off-by: Zixuan Liu <[email protected]> ### Motivation `Message<PulsarEvent>.getValue()` sometimes returns `null` in `SystemTopicBasedTopicPoliciesService.notifyListener()`, so we need to skip the messages that do not belong to the policy type, this problem can cause the policy service to fail to work. ### Modifications - Checks the `Message<PulsarEvent>.getValue()` value. - Uses the `event` instead of `null` as message value when delete policy. ### Verifying this change - [x] Make sure that the change passes the CI checks. ### Does this pull request potentially affect one of the following parts: *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): no - The public API: no - The schema: no - The default values of configurations: no - The wire protocol: no - The rest endpoints: no - The admin cli options: no - Anything that affects deployment: no ### Documentation Check the box below and label this PR (if you have committer privilege). Need to update docs? - [ ] doc-required - [x] no-need-doc - [ ] doc (cherry picked from commit d310e79)
nodece
added a commit
to streamnative/pulsar-archived
that referenced
this pull request
Jan 12, 2022
…#12367) Signed-off-by: Zixuan Liu <[email protected]> ### Motivation `Message<PulsarEvent>.getValue()` sometimes returns `null` in `SystemTopicBasedTopicPoliciesService.notifyListener()`, so we need to skip the messages that do not belong to the policy type, this problem can cause the policy service to fail to work. ### Modifications - Checks the `Message<PulsarEvent>.getValue()` value. - Uses the `event` instead of `null` as message value when delete policy. ### Verifying this change - [x] Make sure that the change passes the CI checks. ### Does this pull request potentially affect one of the following parts: *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): no - The public API: no - The schema: no - The default values of configurations: no - The wire protocol: no - The rest endpoints: no - The admin cli options: no - Anything that affects deployment: no ### Documentation Check the box below and label this PR (if you have committer privilege). Need to update docs? - [ ] doc-required - [x] no-need-doc - [ ] doc (cherry picked from commit d310e79)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
area/broker
cherry-picked/branch-2.8
Archived: 2.8 is end of life
cherry-picked/branch-2.9
Archived: 2.9 is end of life
doc-not-needed
Your PR changes do not impact docs
release/2.8.2
release/2.9.2
type/bug
The PR fixed a bug or issue reported a bug
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Zixuan Liu [email protected]
Motivation
Message<PulsarEvent>.getValue()
sometimes returnsnull
inSystemTopicBasedTopicPoliciesService.notifyListener()
, so we need to skip the messages that do not belong to the policy type, this problem can cause the policy service to fail to work.Modifications
Message<PulsarEvent>.getValue()
value.event
instead ofnull
as message value when delete policy.Verifying this change
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?