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 #4273] Fix DLedgerRpcNettyService can not communicate with each other when tls enabled #4274

Closed
wants to merge 1 commit into from

Conversation

zhanxuefeng
Copy link

Make sure set the target branch to develop

What is the purpose of the change

Fix DLedgerRpcNettyService can not communicate with each other when tls enabled
ISSUE #4273

Brief changelog

NettyRemotingClient load the tls conf from system properties

@zhanxuefeng zhanxuefeng changed the title Fix DLedgerRpcNettyService can not communicate with each other when tls enabled [ISSUE #4273] Fix DLedgerRpcNettyService can not communicate with each other when tls enabled May 10, 2022
@hzh0425
Copy link
Member

hzh0425 commented May 10, 2022

Oh, what a careful discovery.
For optino 'useTLS' of NettyClientConfig, the configuration in BrokerStartup # createBrokerController is as follows:
image
But Dledger # DledgerRpcNettyService directly ignores this option, it should be missed, in addition to this pr, maybe you should initiate a pr in the dledger community to supplement.
image

hzh0425
hzh0425 previously approved these changes May 10, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 10, 2022

Codecov Report

Merging #4274 (4a0f258) into develop (446b76b) will increase coverage by 0.57%.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##             develop    #4274      +/-   ##
=============================================
+ Coverage      47.61%   48.18%   +0.57%     
- Complexity      4958     5111     +153     
=============================================
  Files            633      646      +13     
  Lines          42577    42911     +334     
  Branches        5589     5611      +22     
=============================================
+ Hits           20273    20678     +405     
+ Misses         19796    19724      -72     
- Partials        2508     2509       +1     
Impacted Files Coverage Δ
...e/rocketmq/remoting/netty/NettyRemotingClient.java 46.04% <0.00%> (-0.57%) ⬇️
...apache/rocketmq/broker/client/ProducerManager.java 71.81% <0.00%> (-13.74%) ⬇️
...broker/processor/AbstractSendMessageProcessor.java 40.62% <0.00%> (-8.68%) ⬇️
...he/rocketmq/remoting/protocol/RemotingCommand.java 70.32% <0.00%> (-8.30%) ⬇️
...in/java/org/apache/rocketmq/test/util/MQAdmin.java 38.88% <0.00%> (-5.56%) ⬇️
...g/apache/rocketmq/remoting/netty/NettyEncoder.java 46.15% <0.00%> (-3.85%) ⬇️
...n/java/org/apache/rocketmq/store/RunningFlags.java 31.11% <0.00%> (-2.23%) ⬇️
...che/rocketmq/acl/plain/PlainPermissionManager.java 74.50% <0.00%> (-1.67%) ⬇️
...lient/impl/consumer/DefaultMQPushConsumerImpl.java 40.84% <0.00%> (-1.62%) ⬇️
...mq/client/impl/consumer/RebalanceLitePullImpl.java 72.05% <0.00%> (-1.48%) ⬇️
... and 111 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 446b76b...4a0f258. Read the comment docs.

li-xiao-shuang
li-xiao-shuang previously approved these changes May 12, 2022
HScarb
HScarb previously approved these changes Jun 6, 2022
@hzh0425
Copy link
Member

hzh0425 commented Jun 9, 2022

Can you re-trigger ci test? @zhanxuefeng

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 52.096% when pulling 4a0f258 on zhanxuefeng:dledger into 73b9ac8 on apache:develop.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 52.096% when pulling 4a0f258 on zhanxuefeng:dledger into 73b9ac8 on apache:develop.

@zhanxuefeng
Copy link
Author

Can you re-trigger ci test? @zhanxuefeng

done

@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 Aug 29, 2023
@github-actions
Copy link

github-actions bot commented Sep 1, 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 Sep 1, 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.

7 participants