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] Print warn log if compaction failure #19405

Merged
merged 1 commit into from
Feb 4, 2023

Conversation

poorbarcode
Copy link
Contributor

Motivation

Now exceptions that happen in the flow of compaction task are collected into variable currentCompaction and don't print any log.

When compaction doesn't work, two reasons come to mind:

  • the task skipped by rules.
  • the task failed.

We can't determine which causing the problem if there has no log, this increases the difficulty of troubleshooting.

The API pulsar-admin topics compaction-status <topic> lets us see what happens when the last compaction executes, But it can not help us know if there were previous missions that failed.

Modifications

Print warn log if compaction fails.

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 Feb 2, 2023
@dave2wave dave2wave requested a review from dlg99 February 2, 2023 16:33
@Technoboy- Technoboy- added this to the 3.0.0 milestone Feb 3, 2023
@Technoboy- Technoboy- closed this Feb 3, 2023
@Technoboy- Technoboy- reopened this Feb 3, 2023
@poorbarcode
Copy link
Contributor Author

/pulsarbot rerun-failure-checks

@codecov-commenter
Copy link

Codecov Report

Merging #19405 (6cb2ff6) into master (39dd1cd) will increase coverage by 30.22%.
The diff coverage is 76.31%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #19405       +/-   ##
=============================================
+ Coverage     32.36%   62.59%   +30.22%     
- Complexity     6355    25663    +19308     
=============================================
  Files          1648     1830      +182     
  Lines        123990   133944     +9954     
  Branches      13523    14737     +1214     
=============================================
+ Hits          40129    83838    +43709     
+ Misses        77958    42381    -35577     
- Partials       5903     7725     +1822     
Flag Coverage Δ
inttests 24.88% <71.05%> (+0.07%) ⬆️
systests 25.58% <18.42%> (-0.11%) ⬇️
unittests 59.78% <42.10%> (+42.24%) ⬆️

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

Impacted Files Coverage Δ
...va/org/apache/pulsar/broker/service/ServerCnx.java 54.24% <53.84%> (+17.13%) ⬆️
.../org/apache/pulsar/broker/admin/AdminResource.java 74.93% <87.50%> (+31.90%) ⬆️
...sar/broker/service/persistent/PersistentTopic.java 64.30% <100.00%> (+13.02%) ⬆️
...ker/loadbalance/impl/LeastLongTermMessageRate.java 73.33% <0.00%> (-11.12%) ⬇️
...r/service/AbstractDispatcherMultipleConsumers.java 70.96% <0.00%> (-9.68%) ⬇️
...r/service/schema/DefaultSchemaRegistryService.java 0.00% <0.00%> (-6.25%) ⬇️
...in/java/org/apache/pulsar/PulsarBrokerStarter.java 28.14% <0.00%> (-3.00%) ⬇️
.../apache/pulsar/broker/loadbalance/LoadManager.java 58.33% <0.00%> (-2.78%) ⬇️
.../apache/pulsar/client/impl/ClientCnxIdleState.java 68.88% <0.00%> (-2.23%) ⬇️
...xtensions/channel/ServiceUnitStateChannelImpl.java 0.96% <0.00%> (-0.01%) ⬇️
... and 1224 more

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.

3 participants