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

[ISSUE #8974] Support recalling of delay message #8975

Merged
merged 8 commits into from
Dec 9, 2024

Conversation

imzs
Copy link
Contributor

@imzs imzs commented Nov 22, 2024

[ISSUE #8974] Support recalling of delay message

@qianye1001
Copy link
Contributor

Returning an additional handle in all send Response operations introduces overhead. If this feature is not required, there should be an option to disable it.

@imzs
Copy link
Contributor Author

imzs commented Nov 22, 2024

Returning an additional handle in all send Response operations introduces overhead. If this feature is not required, there should be an option to disable it.

This field is somewhat similar to transactionId. As previously mentioned, the API defines a common semantic. In cases where unsupported message types are encountered, it returns null values, which also implies that the current message does not support recall operation, this may be considered as a switch.

RongtongJin
RongtongJin previously approved these changes Nov 26, 2024
@RongtongJin
Copy link
Contributor

It seems that the newly added integration test are failing.
image

@imzs
Copy link
Contributor Author

imzs commented Nov 28, 2024

It seems that the newly added integration test are failing. image

The tests passed locally, it maybe timeout issue; I'll try to fix it.

…on test, disable polling and wait more time.
@codecov-commenter
Copy link

codecov-commenter commented Nov 28, 2024

Codecov Report

Attention: Patch coverage is 66.31016% with 126 lines in your changes missing coverage. Please review.

Project coverage is 47.85%. Comparing base (715dd5a) to head (7750db7).
Report is 14 commits behind head on develop.

Files with missing lines Patch % Lines
...ient/trace/hook/DefaultRecallMessageTraceHook.java 12.90% 27 Missing ⚠️
...apache/rocketmq/client/trace/TraceDataEncoder.java 0.00% 22 Missing ⚠️
...ng/protocol/header/RecallMessageRequestHeader.java 0.00% 16 Missing ⚠️
...cketmq/proxy/grpc/v2/GrpcMessagingApplication.java 0.00% 11 Missing ⚠️
...ketmq/broker/processor/RecallMessageProcessor.java 89.47% 8 Missing and 2 partials ⚠️
...g/protocol/header/RecallMessageResponseHeader.java 0.00% 5 Missing ⚠️
...ing/protocol/header/SendMessageResponseHeader.java 28.57% 4 Missing and 1 partial ⚠️
...g/apache/rocketmq/client/impl/MQClientAPIImpl.java 81.25% 3 Missing ⚠️
.../java/org/apache/rocketmq/common/BrokerConfig.java 25.00% 3 Missing ⚠️
...tmq/proxy/service/message/LocalMessageService.java 82.35% 3 Missing ⚠️
... and 16 more
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #8975      +/-   ##
=============================================
+ Coverage      47.66%   47.85%   +0.19%     
- Complexity     11766    11861      +95     
=============================================
  Files           1304     1312       +8     
  Lines          91174    91745     +571     
  Branches       11711    11761      +50     
=============================================
+ Hits           43456    43907     +451     
- Misses         42346    42437      +91     
- Partials        5372     5401      +29     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@lollipopjin lollipopjin left a comment

Choose a reason for hiding this comment

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

LGTM

@lollipopjin lollipopjin merged commit bfb3d17 into apache:develop Dec 9, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants