optimize: Use shared EventLoop for TM and RM clients to reduce thread overhead and improve performance
#7179
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 2.x #7179 +/- ##
============================================
+ Coverage 51.55% 51.57% +0.01%
- Complexity 6804 6813 +9
============================================
Files 1169 1169
Lines 41463 41478 +15
Branches 4850 4851 +1
============================================
+ Hits 21375 21391 +16
+ Misses 18069 18065 -4
- Partials 2019 2022 +3
|
| private final NettyPoolKey.TransactionRole transactionRole; | ||
| private final EventLoopGroup sharedEventLoopGroup; |
There was a problem hiding this comment.
I think it should still be named eventLoopGroupWorker, because it is possible that this EventLoopGroup is not shared.
There was a problem hiding this comment.
Done in 982f2a3 !
There was a problem hiding this comment.
I would appreciate it if you could also review the comments below. 🙂
| private final NettyPoolKey.TransactionRole transactionRole; | ||
| private final EventLoopGroup eventLoopGroupWorker; |
There was a problem hiding this comment.
This is not a static variable, the clientBootstrap held by the two instances of TmNettyRemotingClient and RmNettyRemotingClient are not the same, how does this variable manage to be shared use?
There was a problem hiding this comment.
Could this be related to the comment?
In fact, I also believe that it should be a static variable to use the same value across multiple instances.
However, if it becomes a static variable, there’s an issue where the previous eventLoopGroupWorker gets modified when a new one is created. I wanted to ask about that.
There was a problem hiding this comment.
The previous eventLoopGroupWorker should not have been created yet, so I don't think there should be an action to modify this.
There was a problem hiding this comment.
Done in f308be6 !
PTAL ⚡️
Issue: apache#6774
Issue: apache#6774
YongGoose
changed the title
shared EventLoop for TM and RM clients to reduce thre…EventLoop for TM and RM clients to reduce thread overhead and improve performance
Issue: apache#6774
| */ | ||
| public class NettyClientBootstrap implements RemotingBootstrap { | ||
|
|
||
| private static final Logger LOGGER = LoggerFactory.getLogger(NettyClientBootstrap.class); | ||
| private static final String THREAD_PREFIX_SPLIT_CHAR = "_"; | ||
|
|
||
| private static EventLoopGroup eventLoopGroupWorker; |
There was a problem hiding this comment.
I think it should be a static variable, sharedEventLoopGroupWorker, and eventLoopGroupWorker is an instance variable. When enableClientSharedEventLoop is true, sharedEventLoopGroupWorker is created and assigned to eventLoopGroupWorker.
…ad overhead
Ⅰ. Describe what this PR did
Ⅱ. Does this pull request fix one issue?
fixes #6774
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews