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.

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

Open
wants to merge 4 commits into
base: branch-3.0
Choose a base branch
from

Conversation

nodece
Copy link
Member

@nodece nodece commented Jun 24, 2025

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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants