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

[improve][broker] Save createIfMissing in TopicLoadingContext #19993

Conversation

lifepuzzlefun
Copy link
Contributor

@lifepuzzlefun lifepuzzlefun commented Apr 1, 2023

Motivation

When topic load fail to acquire semaphore. a TopicLoadingContext is push into pendingLoadQueue, current TopicLoadingContext should also save the createIfMissing param in the TopicLoadingContext.

Modifications

add field in TopicLoadingContext.

Verifying this change

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:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 1, 2023
@Technoboy- Technoboy- added this to the 3.1.0 milestone Apr 14, 2023
Copy link
Contributor

@poorbarcode poorbarcode left a comment

Choose a reason for hiding this comment

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

Good catch

@poorbarcode
Copy link
Contributor

@lifepuzzlefun Could you write a test?

@lifepuzzlefun
Copy link
Contributor Author

@lifepuzzlefun Could you write a test?

yeah, i'll add one later : - )

@lifepuzzlefun
Copy link
Contributor Author

unit test added : - )

@Technoboy- Technoboy- force-pushed the save_create_if_missing_in_pending_context branch from 928c168 to cf14420 Compare June 9, 2023 03:09
@codecov-commenter
Copy link

Codecov Report

Merging #19993 (cf14420) into master (c39f7bb) will increase coverage by 0.05%.
The diff coverage is 76.47%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19993      +/-   ##
============================================
+ Coverage     72.91%   72.96%   +0.05%     
- Complexity    31916    31925       +9     
============================================
  Files          1867     1867              
  Lines        138558   138561       +3     
  Branches      15217    15219       +2     
============================================
+ Hits         101026   101104      +78     
+ Misses        29501    29435      -66     
+ Partials       8031     8022       -9     
Flag Coverage Δ
inttests 24.19% <0.00%> (+0.01%) ⬆️
systests 25.16% <0.00%> (+0.21%) ⬆️
unittests 72.23% <76.47%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
...ache/pulsar/broker/namespace/NamespaceService.java 70.85% <ø> (+0.50%) ⬆️
.../java/org/apache/pulsar/admin/cli/CmdClusters.java 73.07% <0.00%> (+0.92%) ⬆️
...ain/java/org/apache/pulsar/admin/cli/CmdSinks.java 43.46% <0.00%> (+0.28%) ⬆️
.../apache/pulsar/broker/admin/impl/PackagesBase.java 76.63% <50.00%> (+0.44%) ⬆️
...java/org/apache/pulsar/broker/rest/TopicsBase.java 58.31% <50.00%> (-0.14%) ⬇️
...xtensions/channel/ServiceUnitStateChannelImpl.java 84.43% <100.00%> (+0.05%) ⬆️
...rg/apache/pulsar/broker/service/BrokerService.java 81.69% <100.00%> (+1.04%) ⬆️
...java/org/apache/pulsar/admin/cli/CmdFunctions.java 46.20% <100.00%> (-0.19%) ⬇️
...s/management/core/impl/PackagesManagementImpl.java 77.85% <100.00%> (+4.28%) ⬆️

... and 54 files with indirect coverage changes

@Technoboy- Technoboy- merged commit 45d882a into apache:master Jun 9, 2023
43 checks passed
@RobertIndie
Copy link
Member

From my understanding, this is more like an improvement than a bug fix. We shouldn't put it into branch-3.0. So I remove the label release/3.0.1 here.

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jul 17, 2023
…ssing in TopicLoadingContext (apache#19993)

(cherry picked from commit 043ecd7)
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.

None yet

5 participants