Skip to content

Conversation

@BewareMyPower
Copy link
Contributor

Motivation

Fix the breaking changes introduced from #24118. See https://lists.apache.org/thread/llobypz1nc9w6wdnwd3z1h52z7bygdby for details.

Modifications

First, add a validatePartitionMetadataConsistency option to adopt the behavior introduced in #24118 when it's enabled. This option is disabled by default.

Then, restore all previous tests that are modified in #24118, including

  • AdminApiTest#testPersistentTopicsExpireMessagesInvalidPartitionIndex
  • TopicAutoCreationTest#testPartitionedTopicAutoCreation
  • PulsarClientBasedHandlerTest
  • PersistentTopicTest#testDeleteTopicDeleteOnMetadataStoreFailed
  • ReplicatorTest#testReplicatorOnPartitionedTopic
  • NonPersistentTopicTest#testCreateNonExistentPartitions
  • PersistentTopicTest#testCompatibilityWithPartitionKeyword

Move all new tests to ValidatePartitionMetadataConsistencyTest with validatePartitionMetadataConsistency configured as true.

Note: we need a formal PIP to determine what's the most proper solution. This option is a temporary workaround as a quick fix.

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: BewareMyPower#41

@BewareMyPower BewareMyPower added type/bug The PR fixed a bug or issue reported a bug release/3.0.12 release/3.3.7 release/4.0.5 labels Apr 22, 2025
@BewareMyPower BewareMyPower self-assigned this Apr 22, 2025
@github-actions github-actions bot added the doc-complete Your PR changes impact docs and the related docs have been already added. label Apr 22, 2025
@codecov-commenter
Copy link

codecov-commenter commented Apr 23, 2025

Codecov Report

Attention: Patch coverage is 87.93103% with 7 lines in your changes missing coverage. Please review.

Project coverage is 74.25%. Comparing base (bbc6224) to head (4961f51).
Report is 1033 commits behind head on master.

Files with missing lines Patch % Lines
...rg/apache/pulsar/broker/service/BrokerService.java 87.71% 3 Missing and 4 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24195      +/-   ##
============================================
+ Coverage     73.57%   74.25%   +0.67%     
+ Complexity    32624    32534      -90     
============================================
  Files          1877     1865      -12     
  Lines        139502   144699    +5197     
  Branches      15299    16530    +1231     
============================================
+ Hits         102638   107443    +4805     
+ Misses        28908    28770     -138     
- Partials       7956     8486     +530     
Flag Coverage Δ
inttests 26.74% <46.55%> (+2.16%) ⬆️
systests 23.24% <15.51%> (-1.08%) ⬇️
unittests 73.72% <87.93%> (+0.88%) ⬆️

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

Files with missing lines Coverage Δ
...org/apache/pulsar/broker/ServiceConfiguration.java 98.17% <100.00%> (-1.23%) ⬇️
...rg/apache/pulsar/broker/service/BrokerService.java 84.36% <87.71%> (+3.58%) ⬆️

... and 1074 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.

@lhotari
Copy link
Member

lhotari commented Apr 23, 2025

As per this dev mailing list thread message, I have reverted PRs #24154 and #24118 in branch-3.0, branch-3.3, branch-4.0 .
The plan is to handle the "topic consistency check" feature in Pulsar 4.1 . @nodece will write a PIP for documenting the change.

@BewareMyPower I'll close this PR since we have time to address the details for Pulsar 4.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-complete Your PR changes impact docs and the related docs have been already added. ready-to-test type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants