You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "YongGoose (via GitHub)" <gi...@apache.org> on 2025/02/19 06:06:11 UTC
[PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
YongGoose opened a new pull request, #7170:
URL: https://github.com/apache/incubator-seata/pull/7170
<!-- Please make sure you have read and understood the contributing guidelines -->
- [x] I have registered the PR [changes](../changes).
### Ⅰ. Describe what this PR did
### Ⅱ. Does this pull request fix one issue?
<!-- If that, add "fixes #xxx" below in the next line, for example, fixes #97. -->
fixes #7168
### Ⅲ. Why don't you add test cases (unit test/integration test)?
### Ⅳ. Describe how to verify it
### Ⅴ. Special notes for reviews
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "YongGoose (via GitHub)" <gi...@apache.org>.
YongGoose commented on code in PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#discussion_r1961037072
##########
core/src/main/java/org/apache/seata/core/rpc/netty/NettyClientConfig.java:
##########
@@ -346,11 +345,13 @@ public static String getSocketAddressStartChar() {
/**
* Gets client selector thread size.
+ * If the configured thread size is less than or equal to 0, it returns the default value.
*
- * @return the client selector thread size
+ * @return the client selector thread size, or the default value if the configured size is invalid.
*/
public int getClientSelectorThreadSize() {
- return CONFIG.getInt(ConfigurationKeys.CLIENT_SELECTOR_THREAD_SIZE, DEFAULT_SELECTOR_THREAD_SIZE);
+ int threadSize = CONFIG.getInt(ConfigurationKeys.CLIENT_SELECTOR_THREAD_SIZE, WorkThreadMode.Default.getValue());
+ return threadSize > 0 ? threadSize : WorkThreadMode.Default.getValue();
Review Comment:
@funky-eyes
Handling this in `NettyClientBootstrap` is a good idea, but I believe it is more appropriate to process it here.
Because if we handle this in `NettyClientBootstrap`, we would need to process` -1` in every piece of code that retrieves the thread count using this value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "YongGoose (via GitHub)" <gi...@apache.org>.
YongGoose commented on PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#issuecomment-2667623948
> Then, in the code, handle the value of -1 by converting it to the number of CPU cores * 2.
What do you think about changing the thread count to CPU cores * 2 when the value is negative?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#discussion_r1961014665
##########
core/src/main/java/org/apache/seata/core/rpc/netty/NettyClientConfig.java:
##########
@@ -25,7 +25,6 @@
import static org.apache.seata.common.DefaultValues.DEFAULT_RPC_RM_REQUEST_TIMEOUT;
import static org.apache.seata.common.DefaultValues.DEFAULT_RPC_TM_REQUEST_TIMEOUT;
import static org.apache.seata.common.DefaultValues.DEFAULT_SELECTOR_THREAD_PREFIX;
-import static org.apache.seata.common.DefaultValues.DEFAULT_SELECTOR_THREAD_SIZE;
Review Comment:
The value of DEFAULT_SELECTOR_THREAD_SIZE should be changed to -1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#issuecomment-2667604563
https://github.com/apache/incubator-seata/blob/2.x/script/client/conf/file.conf#L44
https://github.com/apache/incubator-seata/blob/2.x/script/client/spring/application.properties#L74
https://github.com/apache/incubator-seata/blob/2.x/script/client/spring/application.yml#L85
https://github.com/apache/incubator-seata/blob/2.x/script/config-center/config.txt#L35
Since the corresponding IO thread count for the client in the above files is 1, I believe these configurations should be changed to -1. Then, in the code, handle the value of -1 by converting it to the number of CPU cores * 2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "YongGoose (via GitHub)" <gi...@apache.org>.
YongGoose commented on code in PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#discussion_r1961031385
##########
core/src/main/java/org/apache/seata/core/rpc/netty/NettyClientConfig.java:
##########
@@ -25,7 +25,6 @@
import static org.apache.seata.common.DefaultValues.DEFAULT_RPC_RM_REQUEST_TIMEOUT;
import static org.apache.seata.common.DefaultValues.DEFAULT_RPC_TM_REQUEST_TIMEOUT;
import static org.apache.seata.common.DefaultValues.DEFAULT_SELECTOR_THREAD_PREFIX;
-import static org.apache.seata.common.DefaultValues.DEFAULT_SELECTOR_THREAD_SIZE;
Review Comment:
> > Done in [f5d38a9](https://github.com/apache/incubator-seata/commit/f5d38a93014946cf08a563a4b37c44b9e8687b5c) !
>
> In NettyClientBootstrap, it is necessary to handle the case where selectorThreadSizeThreadSize=-1.
- https://github.com/apache/incubator-seata/pull/7170#issuecomment-2667623948
What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#discussion_r1961043996
##########
core/src/main/java/org/apache/seata/core/rpc/netty/NettyClientConfig.java:
##########
@@ -346,11 +345,13 @@ public static String getSocketAddressStartChar() {
/**
* Gets client selector thread size.
+ * If the configured thread size is less than or equal to 0, it returns the default value.
*
- * @return the client selector thread size
+ * @return the client selector thread size, or the default value if the configured size is invalid.
*/
public int getClientSelectorThreadSize() {
- return CONFIG.getInt(ConfigurationKeys.CLIENT_SELECTOR_THREAD_SIZE, DEFAULT_SELECTOR_THREAD_SIZE);
+ int threadSize = CONFIG.getInt(ConfigurationKeys.CLIENT_SELECTOR_THREAD_SIZE, WorkThreadMode.Default.getValue());
+ return threadSize > 0 ? threadSize : WorkThreadMode.Default.getValue();
Review Comment:
You are right, handling it this way is better, good job.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "YongGoose (via GitHub)" <gi...@apache.org>.
YongGoose commented on code in PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#discussion_r1961037072
##########
core/src/main/java/org/apache/seata/core/rpc/netty/NettyClientConfig.java:
##########
@@ -346,11 +345,13 @@ public static String getSocketAddressStartChar() {
/**
* Gets client selector thread size.
+ * If the configured thread size is less than or equal to 0, it returns the default value.
*
- * @return the client selector thread size
+ * @return the client selector thread size, or the default value if the configured size is invalid.
*/
public int getClientSelectorThreadSize() {
- return CONFIG.getInt(ConfigurationKeys.CLIENT_SELECTOR_THREAD_SIZE, DEFAULT_SELECTOR_THREAD_SIZE);
+ int threadSize = CONFIG.getInt(ConfigurationKeys.CLIENT_SELECTOR_THREAD_SIZE, WorkThreadMode.Default.getValue());
+ return threadSize > 0 ? threadSize : WorkThreadMode.Default.getValue();
Review Comment:
@funky-eyes
Handling this in `NettyClientBootstrap` is a good idea, but I believe it is more appropriate to process it here.
Because if we handle this in `NettyClientBootstrap`, we would need to process` -1` in every piece of code that retrieves the thread count using this method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes merged PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "YongGoose (via GitHub)" <gi...@apache.org>.
YongGoose commented on code in PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#discussion_r1961023072
##########
core/src/main/java/org/apache/seata/core/rpc/netty/NettyClientConfig.java:
##########
@@ -25,7 +25,6 @@
import static org.apache.seata.common.DefaultValues.DEFAULT_RPC_RM_REQUEST_TIMEOUT;
import static org.apache.seata.common.DefaultValues.DEFAULT_RPC_TM_REQUEST_TIMEOUT;
import static org.apache.seata.common.DefaultValues.DEFAULT_SELECTOR_THREAD_PREFIX;
-import static org.apache.seata.common.DefaultValues.DEFAULT_SELECTOR_THREAD_SIZE;
Review Comment:
Done in https://github.com/apache/incubator-seata/pull/7170/commits/f5d38a93014946cf08a563a4b37c44b9e8687b5c !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "YongGoose (via GitHub)" <gi...@apache.org>.
YongGoose commented on PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#issuecomment-2667760647
@funky-eyes
I have resolved the failing test related to `getClientSelectorThreadSize`.
However, the `testRemoveConfig` test in `ZkConfigurationTest` is still failing, but only when running on `Java 8`.
Could you help me understand why this failure occurs specifically in `Java 8`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#discussion_r1961027812
##########
core/src/main/java/org/apache/seata/core/rpc/netty/NettyClientConfig.java:
##########
@@ -25,7 +25,6 @@
import static org.apache.seata.common.DefaultValues.DEFAULT_RPC_RM_REQUEST_TIMEOUT;
import static org.apache.seata.common.DefaultValues.DEFAULT_RPC_TM_REQUEST_TIMEOUT;
import static org.apache.seata.common.DefaultValues.DEFAULT_SELECTOR_THREAD_PREFIX;
-import static org.apache.seata.common.DefaultValues.DEFAULT_SELECTOR_THREAD_SIZE;
Review Comment:
> Done in [f5d38a9](https://github.com/apache/incubator-seata/commit/f5d38a93014946cf08a563a4b37c44b9e8687b5c) !
In NettyClientBootstrap, it is necessary to handle the case where selectorThreadSizeThreadSize=-1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#issuecomment-2667638431
> What do you think about changing the thread count to CPU cores * 2 when the value is negative?
If it is a negative number, directly assign the value of `WorkThreadMode.Default.getValue()` to it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#issuecomment-2667607177
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/7170?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 52.18%. Comparing base [(`46c9b63`)](https://app.codecov.io/gh/apache/incubator-seata/commit/46c9b63728e47af2b0a0fab5deb9baef66b1d36a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`0bbd7e9`)](https://app.codecov.io/gh/apache/incubator-seata/commit/0bbd7e92c1aa4f95dcc6ee85a21c2652970277ed?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/incubator-seata/pull/7170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 2.x #7170 +/- ##
=========================================
Coverage 52.18% 52.18%
- Complexity 6813 6817 +4
=========================================
Files 1154 1154
Lines 41112 41112
Branches 4818 4818
=========================================
Hits 21455 21455
- Misses 17617 17620 +3
+ Partials 2040 2037 -3
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/7170?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...apache/seata/core/rpc/netty/NettyClientConfig.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7170?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcore%2Frpc%2Fnetty%2FNettyClientConfig.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29yZS9ycGMvbmV0dHkvTmV0dHlDbGllbnRDb25maWcuamF2YQ==) | `52.94% <100.00%> (ø)` | |
... and [3 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/7170/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize: Optimize seata client I/O processing by adjusting thread count [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #7170:
URL: https://github.com/apache/incubator-seata/pull/7170#issuecomment-2667813143
> @funky-eyes
>
> I have resolved the failing test related to `getClientSelectorThreadSize`. However, the `testRemoveConfig` test in `ZkConfigurationTest` is still failing, but only when running on `Java 8`. Could you help me understand why this failure occurs specifically in `Java 8`?
This is not caused by your PR; this issue is an occasional occurrence, and the exact cause is currently unknown. I have restarted the CI workflow for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org