Skip to content

[ISSUE #4496] Response should not be processed during the NettyRequestProcessor requestProcess, neither by being assigned as NULL nor by being written to the Netty channel.#4723

Closed
Erik1288 wants to merge 6 commits intoapache:developfrom
Erik1288:new-develop-4496

Conversation

@Erik1288
Copy link
Contributor

@Erik1288 Erik1288 commented Jul 29, 2022

Make sure set the target branch to develop

What is the purpose of the change

Here's some cases:

image
When implementing zeroCopy, the response is written to the netty channel even before the processRequest method ends.

image

When implementing long-polling, the response is assigned and returned as NULL instead of returning an asynchronous result.

Brief changelog

  1. Refactoring the processRequest method signatures in concrete Processors by no longer taking channel as a parameter.
  2. Refactoring logic like long-polling and Zerocopy that are not properly processing response during the NettyReqeustProcessor processing.
  3. The NettyRemotingAbstract part is based on [Issue #4437]Possible alternative solution to get flow control capability without changing the RPCHook Interface #4454.

Verifying this change

Pass UTs.

Erik1288 added 6 commits July 29, 2022 11:09
…uestProcess, neither by being assigned as NULL nor by being written to the Netty channel.
…uestProcess, neither by being assigned as NULL nor by being written to the Netty channel.
…uestProcess, neither by being assigned as NULL nor by being written to the Netty channel.
# Conflicts:
#	proxy/src/main/java/org/apache/rocketmq/proxy/service/mqclient/ProxyClientRemotingProcessor.java
@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2022

Codecov Report

Merging #4723 (1d28bb6) into develop (c0e3751) will decrease coverage by 0.01%.
The diff coverage is 32.80%.

@@              Coverage Diff              @@
##             develop    #4723      +/-   ##
=============================================
- Coverage      44.82%   44.81%   -0.02%     
- Complexity      7564     7572       +8     
=============================================
  Files            977      983       +6     
  Lines          67640    67744     +104     
  Branches        8933     8944      +11     
=============================================
+ Hits           30321    30360      +39     
- Misses         33581    33643      +62     
- Partials        3738     3741       +3     
Impacted Files Coverage Δ
...cketmq/broker/longpolling/NotificationRequest.java 0.00% <0.00%> (ø)
...pache/rocketmq/broker/longpolling/PullRequest.java 0.00% <0.00%> (ø)
...tmq/broker/longpolling/PullRequestHoldService.java 16.50% <0.00%> (-0.33%) ⬇️
...ker/processor/DefaultPullMessageResultHandler.java 36.89% <0.00%> (+1.75%) ⬆️
...cketmq/broker/processor/NotificationProcessor.java 10.52% <0.00%> (-0.07%) ⬇️
...ocketmq/broker/processor/PeekMessageProcessor.java 3.67% <0.00%> (+0.07%) ⬆️
...ocketmq/broker/processor/PollingInfoProcessor.java 8.16% <0.00%> (-0.18%) ⬇️
...cketmq/broker/processor/QueryMessageProcessor.java 6.25% <0.00%> (+0.53%) ⬆️
...e/rocketmq/container/BrokerContainerProcessor.java 4.13% <0.00%> (-0.03%) ⬇️
...namesrv/processor/ClusterTestRequestProcessor.java 54.28% <ø> (ø)
... and 53 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 48.844% when pulling 1d28bb6 on Erik1288:new-develop-4496 into c0e3751 on apache:develop.

@github-actions
Copy link

This PR is stale because it has been open for 365 days with no activity. It will be closed in 3 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR.

@github-actions github-actions bot added the stale label Jul 30, 2023
@github-actions
Copy link

github-actions bot commented Aug 3, 2023

This PR was closed because it has been inactive for 3 days since being marked as stale.

@github-actions github-actions bot closed this Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants