Skip to content

Conversation

@jiangpengcheng
Copy link
Contributor

@jiangpengcheng jiangpengcheng commented Sep 9, 2025

Fixes #24720

Motivation

The PulsarMetadataStateStoreProviderImpl.init method fetches the METADATA_URL field from the config argument, but it doesn't exist in normal case, fallback to stateStorageServiceUrl just like other StateStoreProviderImpl

Modifications

When the provided argument doesn't contains a METADATA_URL field, fallback to fetching with key stateStorageServiceUrl

Verifying this change

  • Make sure that the change passes the CI checks.

  • added new integration tests in:

    • org.apache.pulsar.tests.integration.functions.PulsarMetadataStoreStateTest

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: jiangpengcheng#47

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 9, 2025
@lhotari
Copy link
Member

lhotari commented Sep 9, 2025

@jiangpengcheng Do you have a chance to add a new integration test where this problem gets reproduced and where this fix addresses it?

@jiangpengcheng
Copy link
Contributor Author

@jiangpengcheng Do you have a chance to add a new integration test where this problem gets reproduced and where this fix addresses it?

sure, I will add some integration tests

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.19%. Comparing base (a2b69cc) to head (f218f77).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...ce/state/PulsarMetadataStateStoreProviderImpl.java 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24721      +/-   ##
============================================
+ Coverage     74.17%   74.19%   +0.02%     
- Complexity    33470    33502      +32     
============================================
  Files          1895     1896       +1     
  Lines        148029   148111      +82     
  Branches      17142    17164      +22     
============================================
+ Hits         109796   109891      +95     
+ Misses        29479    29446      -33     
- Partials       8754     8774      +20     
Flag Coverage Δ
inttests 26.39% <0.00%> (+0.14%) ⬆️
systests 22.69% <0.00%> (+0.03%) ⬆️
unittests 73.70% <0.00%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
...ce/state/PulsarMetadataStateStoreProviderImpl.java 0.00% <0.00%> (ø)

... and 109 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Technoboy- Technoboy- merged commit 415c6fa into apache:master Sep 10, 2025
51 checks passed
@lhotari
Copy link
Member

lhotari commented Sep 10, 2025

Great work @jiangpengcheng

lhotari pushed a commit that referenced this pull request Sep 11, 2025
…tadataStateStoreProviderImpl.init` (#24721)

(cherry picked from commit 415c6fa)
lhotari pushed a commit that referenced this pull request Sep 11, 2025
…tadataStateStoreProviderImpl.init` (#24721)

(cherry picked from commit 415c6fa)
lhotari pushed a commit that referenced this pull request Sep 11, 2025
…tadataStateStoreProviderImpl.init` (#24721)

(cherry picked from commit 415c6fa)
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Sep 12, 2025
…tadataStateStoreProviderImpl.init` (apache#24721)

(cherry picked from commit 415c6fa)
(cherry picked from commit 3ec5e64)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Sep 12, 2025
…tadataStateStoreProviderImpl.init` (apache#24721)

(cherry picked from commit 415c6fa)
(cherry picked from commit 3ec5e64)
KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] PulsarMetadataStateStoreProviderImpl doesn't work with functions

4 participants