Skip to content

Conversation

@heesung-sohn
Copy link
Contributor

@heesung-sohn heesung-sohn commented Sep 12, 2024

PIP: #23300

Motivation

Implements the PIP: #23300

Modifications

  • Add ServiceUnitStateTableView interface
  • Add ServiceUnitStateTableViewImpl implementation that uses Pulsar System topic (compatible with existing behavior)
  • Add ServiceUnitStateMetadataStoreTableViewImpl implementation that uses Pulsar Metadata Store (new behavior)
  • Refactor ExtensibleLoadMangerImpl and ServiceUnitStateChannelImpl to accept ServiceUnitStateTableView.
  • Add time-based producer restart logic in LoadDataTableView and improved its shutdown logic.
  • Improve ownership release logic to be more graceful upon broker shutdown
  • Refactor related tests code
  • ServiceUnitStateTableViewSyncer to sync the two table views during migration.

Verifying this change

  • Make sure that the change passes the CI checks.

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: heesung-sohn#81

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 12, 2024
@heesung-sohn heesung-sohn force-pushed the elm-tableview branch 4 times, most recently from bd6d9bc to a2e1506 Compare September 12, 2024 23:37
@heesung-sohn heesung-sohn self-assigned this Sep 13, 2024
@heesung-sohn heesung-sohn added this to the 4.0.0 milestone Sep 13, 2024
@heesung-sohn heesung-sohn force-pushed the elm-tableview branch 9 times, most recently from b3fbb20 to 25501d4 Compare September 17, 2024 23:33
@heesung-sohn heesung-sohn force-pushed the elm-tableview branch 5 times, most recently from ed14f2a to bfaae93 Compare September 18, 2024 22:41
@heesung-sohn
Copy link
Contributor Author

@BewareMyPower @Demogorgon314 Updated the code. PTAL thanks.

@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 78.50746% with 144 lines in your changes missing coverage. Please review.

Project coverage is 74.70%. Comparing base (bbc6224) to head (bee4bd5).
Report is 592 commits behind head on master.

Files with missing lines Patch % Lines
...sions/channel/ServiceUnitStateTableViewSyncer.java 68.25% 24 Missing and 16 partials ⚠️
...ata/tableview/impl/MetadataStoreTableViewImpl.java 75.65% 24 Missing and 13 partials ⚠️
...el/ServiceUnitStateMetadataStoreTableViewImpl.java 60.78% 14 Missing and 6 partials ⚠️
...xtensions/channel/ServiceUnitStateChannelImpl.java 81.25% 14 Missing and 4 partials ⚠️
...ensions/channel/ServiceUnitStateTableViewImpl.java 75.67% 11 Missing and 7 partials ⚠️
...lance/extensions/channel/ServiceUnitStateData.java 50.00% 1 Missing and 2 partials ⚠️
...dbalance/extensions/ExtensibleLoadManagerImpl.java 89.47% 1 Missing and 1 partial ⚠️
...ensions/channel/ServiceUnitStateTableViewBase.java 94.44% 2 Missing ⚠️
...er/loadbalance/extensions/store/LoadDataStore.java 0.00% 2 Missing ⚠️
.../pulsar/metadata/cache/impl/MetadataCacheImpl.java 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23301      +/-   ##
============================================
+ Coverage     73.57%   74.70%   +1.13%     
+ Complexity    32624     2757   -29867     
============================================
  Files          1877     1936      +59     
  Lines        139502   145635    +6133     
  Branches      15299    15935     +636     
============================================
+ Hits         102638   108802    +6164     
+ Misses        28908    28540     -368     
- Partials       7956     8293     +337     
Flag Coverage Δ
inttests 27.89% <27.16%> (+3.30%) ⬆️
systests 24.62% <4.77%> (+0.30%) ⬆️
unittests 74.07% <78.50%> (+1.23%) ⬆️

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.96% <100.00%> (-0.43%) ⬇️
...adbalance/extensions/channel/ServiceUnitState.java 100.00% <100.00%> (ø)
.../channel/ServiceUnitStateDataConflictResolver.java 76.00% <100.00%> (ø)
...e/extensions/store/TableViewLoadDataStoreImpl.java 96.77% <100.00%> (+8.67%) ⬆️
...sar/broker/service/persistent/PersistentTopic.java 79.54% <ø> (+1.08%) ⬆️
...pache/pulsar/metadata/api/MetadataCacheConfig.java 83.33% <100.00%> (+3.33%) ⬆️
...he/pulsar/metadata/api/MetadataStoreTableView.java 100.00% <100.00%> (ø)
...dbalance/extensions/ExtensibleLoadManagerImpl.java 83.33% <89.47%> (+3.24%) ⬆️
...ensions/channel/ServiceUnitStateTableViewBase.java 94.44% <94.44%> (ø)
...er/loadbalance/extensions/store/LoadDataStore.java 0.00% <0.00%> (ø)
... and 7 more

... and 579 files with indirect coverage changes

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 PIP ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants