Skip to content

[fix][test][branch-3.0] Correct topic policy loading logic and improve related tests #24451

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
Merged
lhotari merged 4 commits into from
Jul 24, 2025
Merged

[fix][test][branch-3.0] Correct topic policy loading logic and improve related tests #24451

lhotari merged 4 commits into from
Jul 24, 2025

Conversation

@nodece
Copy link
Member

@nodece nodece commented Jun 24, 2025 •
edited
Loading

Motivation

#24279 breaks the branch-3.0 CI.

Modifications

  • Fixed testCheckPersistencePolicies to align with updated topic policy loading behavior.
  • Fixed an issue where topic policies were incorrectly loaded when the load balancer had not yet started.
  • Move ExtensibleLoadManagerCloseTest test groups to flaky, which always fail.

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 Jun 24, 2025
@nodece nodece self-assigned this Jun 24, 2025
@nodece nodece closed this Jun 24, 2025
@nodece nodece reopened this Jun 24, 2025
@nodece nodece requested a review from lhotari June 24, 2025 07:14
@lhotari lhotari changed the title [fix][test] Correct topic policy loading logic and improve related tests [fix][test][branch-3.0] Correct topic policy loading logic and improve related tests Jun 24, 2025
@lhotari
Copy link
Member

lhotari commented Jun 24, 2025

  • Fixed pulsar-sql license.

I'm sorry I missed this. Thanks for fixing. I pushed this commit directly to branch-3.0 so that it doesn't mix the review. 2061f681

@nodece nodece force-pushed the fix-branch-3.0-test branch from 38054c5 to db15cc6 Compare June 24, 2025 10:00
@nodece
Copy link
Member Author

nodece commented Jun 24, 2025

@lhotari You can directly push this pr to the branch-3.0, or use merge button.

@lhotari
Copy link
Member

lhotari commented Jun 24, 2025

  • Move ExtensibleLoadManagerCloseTest and ExtensibleLoadManagerImplTest test groups to flaky.

@nodece Do these tests ever pass or are they completely broken?

@lhotari
Copy link
Member

lhotari commented Jun 24, 2025

@lhotari You can directly push this pr to the branch-3.0, or use merge button.

@nodece It makes sense to merge this PR. I just have one question about the tests that are moved to flaky group. In master branch they are also in the flaky group, however they pass in most cases.

@nodece
Copy link
Member Author

nodece commented Jun 25, 2025

  • Move ExtensibleLoadManagerCloseTest and ExtensibleLoadManagerImplTest test groups to flaky.

@nodece Do these tests ever pass or are they completely broken?

Some tests have failed:

Error:  Failures: 
  Error:  org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerCloseTest.testCloseAfterLoadingBundles
  [INFO]   Run 1: PASS
  Error:    Run 2: ExtensibleLoadManagerCloseTest.testCloseAfterLoadingBundles:90 » PulsarAdmin java.lang.IllegalStateException: Client instance has been closed.
  [INFO] 
  Error:  org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerImplTest.testNamespaceOwnershipListener
  [INFO]   Run 1: PASS
  Error:    Run 2: ExtensibleLoadManagerImplTest.testNamespaceOwnershipListener:436 » ConditionTimeout Assertion condition defined as a org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerImplTest expected [1] but found [2] within 10 seconds.
  [INFO] 
  Error:  org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerImplTest.testRoleChange

I don't have time to handle the ExtensibleLoadManager test, because these test has been fixed by some feature, we should not cherry-pick feature to the branch-3.0, so like: #23301

@nodece nodece force-pushed the fix-branch-3.0-test branch from db15cc6 to 9f734fd Compare June 25, 2025 03:23
@nodece
Copy link
Member Author

nodece commented Jun 25, 2025

Now, ExtensibleLoadManagerImplTest belongs to the broker test group.

@nodece nodece requested a review from codelipenghui June 26, 2025 01:54
nodece added 4 commits July 24, 2025 11:37
@nodece
[fix][test] Fix testCheckPersistencePolicies
4ea0f53 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece
[fix][test] Fix topic policies loading
0f0cb7b 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece
[fix][test] Move ExtensibleLoadManagerCloseTest test group to flaky
92b54fe 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece
[fix][test] Fix testNamespaceOwnershipListener
b9f36d2 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece nodece force-pushed the fix-branch-3.0-test branch from 5eb9f71 to b9f36d2 Compare July 24, 2025 04:30
nodece
nodece commented Jul 24, 2025
View reviewed changes
...est/java/org/apache/pulsar/broker/loadbalance/extensions/ExtensibleLoadManagerCloseTest.java

@Slf4j
@Test(groups = "flaky")
public class ExtensibleLoadManagerCloseTest {
Copy link
Member Author

@nodece nodece Jul 24, 2025

Choose a reason for hiding this comment

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

I don't have time to fix this test

lhotari
lhotari approved these changes Jul 24, 2025
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.

Thanks for fixing the problem @nodece

@lhotari lhotari merged commit 91a40e2 into apache:branch-3.0 Jul 24, 2025
83 of 86 checks passed
@nodece nodece deleted the fix-branch-3.0-test branch July 24, 2025 14:34
lhotari added a commit that referenced this pull request Jul 24, 2025
@lhotari
[fix][test][branch-3.3] Fix ExtensibleLoadManager related tests block…
12fe9f3 
…ing release

backports #24451 commit 91a40e2 from branch-3.0 to branch-3.3
@lhotari
Copy link
Member

lhotari commented Jul 24, 2025

I backported this to branch-3.3 in 12fe9f3. There were slight differences in branch-3.3.

nodece added a commit to ascentstream/pulsar that referenced this pull request Jul 28, 2025
@nodece
[fix][test][branch-3.0] Correct topic policy loading logic and improv…
50e8964 
…e related tests (apache#24451)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
nodece added a commit to ascentstream/pulsar that referenced this pull request Jul 28, 2025
@nodece
[fix][test][branch-3.0] Correct topic policy loading logic and improv…
a7c87bf 
…e related tests (apache#24451)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
priyanshu-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 28, 2025
@nodece @priyanshu-ctds
[fix][test][branch-3.0] Correct topic policy loading logic and improv…
78430d4 
…e related tests (apache#24451)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 91a40e2)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 31, 2025
@nodece @srinath-ctds
[fix][test][branch-3.0] Correct topic policy loading logic and improv…
e76eefd 
…e related tests (apache#24451)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 91a40e2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lhotari lhotari lhotari approved these changes

@codelipenghui codelipenghui Awaiting requested review from codelipenghui

@dao-jun dao-jun Awaiting requested review from dao-jun

@poorbarcode poorbarcode Awaiting requested review from poorbarcode

@liudezhi2098 liudezhi2098 Awaiting requested review from liudezhi2098

@BewareMyPower BewareMyPower Awaiting requested review from BewareMyPower

Assignees

@nodece nodece

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nodece @lhotari