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.

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][meta] fix getChildren in MemoryMetadataStore and EtcdMetadataStore #18172

Merged
merged 1 commit into from
Oct 25, 2022

Conversation

coderzc
Copy link
Member

@coderzc coderzc commented Oct 24, 2022

Motivation

In the current implementation, the getChildren will return incorrect results when getting children paths of / and some special path in MemoryMetadataStore and EtcdMetadataStore, because we use replace lead to the basePath has been replaced many times.

Modifications

Use replaceFirst instead of replace.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as testGetChildren.

Documentation

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

Matching PR in forked repository

PR in forked repository: coderzc#16

@coderzc coderzc changed the title [fix][meta] fix getChildren in memoryMetadata and etcdMetadata [fix][meta] fix getChildren in MemoryMetadataStore and EtcdMetadataStore Oct 24, 2022
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 24, 2022
@Technoboy- Technoboy- added this to the 2.12.0 milestone Oct 24, 2022
@Technoboy- Technoboy- added type/bug The PR fixed a bug or issue reported a bug release/2.11.1 release/2.10.3 ready-to-test labels Oct 24, 2022
@Technoboy- Technoboy- closed this Oct 24, 2022
@Technoboy- Technoboy- reopened this Oct 24, 2022
@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2022

Codecov Report

Merging #18172 (40d4456) into master (6c65ca0) will increase coverage by 10.87%.
The diff coverage is 53.63%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #18172       +/-   ##
=============================================
+ Coverage     34.91%   45.78%   +10.87%     
- Complexity     5707    17608    +11901     
=============================================
  Files           607     1579      +972     
  Lines         53396   128710    +75314     
  Branches       5712    14153     +8441     
=============================================
+ Hits          18644    58932    +40288     
- Misses        32119    63748    +31629     
- Partials       2633     6030     +3397     
Flag Coverage Δ
unittests 45.78% <53.63%> (+10.87%) ⬆️

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

Impacted Files Coverage Δ
...org/apache/pulsar/broker/ServiceConfiguration.java 52.06% <ø> (ø)
.../service/SystemTopicBasedTopicPoliciesService.java 62.57% <0.00%> (+10.98%) ⬆️
.../pulsar/broker/stats/BrokerOperabilityMetrics.java 98.21% <ø> (+5.56%) ⬆️
...g/apache/pulsar/compaction/CompactedTopicImpl.java 69.28% <0.00%> (+58.57%) ⬆️
...va/org/apache/pulsar/client/impl/ConsumerBase.java 21.95% <0.00%> (ø)
...ache/pulsar/functions/utils/io/ConnectorUtils.java 0.00% <0.00%> (ø)
.../org/apache/pulsar/broker/admin/v2/Namespaces.java 56.85% <50.00%> (+48.82%) ⬆️
...he/pulsar/functions/worker/rest/api/SinksImpl.java 36.82% <50.00%> (ø)
...apache/pulsar/proxy/server/DirectProxyHandler.java 63.63% <50.00%> (ø)
...ulsar/functions/worker/rest/api/ComponentImpl.java 25.26% <57.14%> (ø)
... and 1149 more

Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM, good catch.

@coderzc
Copy link
Member Author

coderzc commented Oct 25, 2022

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit 4ffa741 into apache:master Oct 25, 2022
liangyepianzhou pushed a commit that referenced this pull request Dec 14, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jan 10, 2023
…tore (apache#18172)

(cherry picked from commit 4ffa741)
(cherry picked from commit 3792d63)
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jan 11, 2023
…tore (apache#18172)

(cherry picked from commit 4ffa741)
(cherry picked from commit 3792d63)
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.

7 participants