You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/04/09 04:12:23 UTC
[GitHub] [rocketmq] dugenkui03 opened a new pull request, #4138: `RemotingCommand` is not thread-safe
dugenkui03 opened a new pull request, #4138:
URL: https://github.com/apache/rocketmq/pull/4138
**Make sure set the target branch to `develop`**
## What is the purpose of the change
When the `HashMap` state is modified by one thread, another thread could call `get…`.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #4138: `RemotingCommand` is not thread-safe
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-1093680830
[](https://coveralls.io/builds/48139665)
Coverage decreased (-0.02%) to 51.963% when pulling **749825073086c4f4be310f0c3789998417b728a4 on dugenkui03:patch-14** into **47561bdd855ab381113d736b5c26a09c08da590b on apache:develop**.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] lwclover commented on pull request #4138: `RemotingCommand` is not thread-safe
Posted by GitBox <gi...@apache.org>.
lwclover commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-1096562505
> > Why not use ConcurrentHashMap instead of HashMap？Using ConcurrentHashMap, no need to lock when reading, and the scope of locking is smaller when writing.
>
> That's my opinion: Do as little as possible to fix mistakes.
>
> # describe in Chinese
> 我习惯做尽可能少的事情去修正bug，尽量不去破坏原有的设计。
>
> 这里使用`synchronized`的原因也许是jvm对`synchronized`加锁进行了优化：如果加锁代码块没有真实的发生多线程并发访问，则永远不会真正的加锁。我简单跟踪了下代码，应该是这个原因。 <img alt="image" width="1177" src="https://user-images.githubusercontent.com/18216266/162627024-8459f20e-ec9c-4e75-bac4-da0c5140ab8d.png">
You were right. Synchronized has a lock upgrade process,Biased locks -> Lightweight locks -> Heavyweight locks.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] lizhanhui commented on pull request #4138: `RemotingCommand` is not thread-safe
Posted by GitBox <gi...@apache.org>.
lizhanhui commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-1150615894
@dugenkui03 this PR was closed without merge. I do not see why.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] `RemotingCommand` is not thread-safe [rocketmq]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-2071152429
This PR was closed because it has been inactive for 3 days since being marked as stale.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] zhouxinyu commented on pull request #4138: `RemotingCommand` is not thread-safe
Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-1150623942
> Why not use ConcurrentHashMap instead of HashMap？Using ConcurrentHashMap, no need to lock when reading, and the scope of locking is smaller when writing.
+1 for using ConcurrentHashMap to fix this issue
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] `RemotingCommand` is not thread-safe [rocketmq]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-2067411405
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.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dugenkui03 commented on pull request #4138: `RemotingCommand` is not thread-safe
Posted by GitBox <gi...@apache.org>.
dugenkui03 commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-1150879469
@lizhanhui Thanks for you reopen this PR. I deleted original fork by mistake, and lots of pr were closed automatically. All the closed pr will reopen and finished.
> 误删除了之前的fork、所有的pr都自动关闭了。近期会重新处理因为误删除fork导致的自动关闭的pr。
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] codecov-commenter commented on pull request #4138: `RemotingCommand` is not thread-safe
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-1093679182
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4138?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#4138](https://codecov.io/gh/apache/rocketmq/pull/4138?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7498250) into [develop](https://codecov.io/gh/apache/rocketmq/commit/fd554ab12072225325c957ff6bdf492fc67821af?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fd554ab) will **decrease** coverage by `0.01%`.
> The diff coverage is `85.71%`.
```diff
@@ Coverage Diff @@
## develop #4138 +/- ##
=============================================
- Coverage 47.92% 47.90% -0.02%
+ Complexity 5002 5000 -2
=============================================
Files 634 635 +1
Lines 42529 42501 -28
Branches 5573 5568 -5
=============================================
- Hits 20381 20362 -19
+ Misses 19647 19639 -8
+ Partials 2501 2500 -1
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/rocketmq/remoting/protocol/RemotingCommand.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL3Byb3RvY29sL1JlbW90aW5nQ29tbWFuZC5qYXZh) | `77.95% <85.71%> (-0.27%)` | :arrow_down: |
| [...in/java/org/apache/rocketmq/test/util/MQAdmin.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW4uamF2YQ==) | `38.88% <0.00%> (-5.56%)` | :arrow_down: |
| [...ketmq/common/protocol/body/ConsumerConnection.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9Db25zdW1lckNvbm5lY3Rpb24uamF2YQ==) | `95.83% <0.00%> (-4.17%)` | :arrow_down: |
| [...ava/org/apache/rocketmq/test/util/VerifyUtils.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL1ZlcmlmeVV0aWxzLmphdmE=) | `46.26% <0.00%> (-2.99%)` | :arrow_down: |
| [...ava/org/apache/rocketmq/filter/util/BitsArray.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmlsdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9maWx0ZXIvdXRpbC9CaXRzQXJyYXkuamF2YQ==) | `59.82% <0.00%> (-2.57%)` | :arrow_down: |
| [...va/org/apache/rocketmq/logging/inner/Appender.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bG9nZ2luZy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbG9nZ2luZy9pbm5lci9BcHBlbmRlci5qYXZh) | `34.83% <0.00%> (-2.25%)` | :arrow_down: |
| [...mq/client/impl/consumer/RebalanceLitePullImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9SZWJhbGFuY2VMaXRlUHVsbEltcGwuamF2YQ==) | `72.05% <0.00%> (-1.48%)` | :arrow_down: |
| [...rg/apache/rocketmq/remoting/netty/NettyLogger.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5TG9nZ2VyLmphdmE=) | `14.96% <0.00%> (-1.37%)` | :arrow_down: |
| [...e/rocketmq/client/impl/consumer/RebalanceImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9SZWJhbGFuY2VJbXBsLmphdmE=) | `43.75% <0.00%> (-1.18%)` | :arrow_down: |
| [.../apache/rocketmq/logging/inner/LoggingBuilder.java](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bG9nZ2luZy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbG9nZ2luZy9pbm5lci9Mb2dnaW5nQnVpbGRlci5qYXZh) | `63.60% <0.00%> (-1.11%)` | :arrow_down: |
| ... and [18 more](https://codecov.io/gh/apache/rocketmq/pull/4138/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/4138?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4138?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fd554ab...7498250](https://codecov.io/gh/apache/rocketmq/pull/4138?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #4138: `RemotingCommand` is not thread-safe
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-1093680825
[](https://coveralls.io/builds/48139665)
Coverage decreased (-0.02%) to 51.963% when pulling **749825073086c4f4be310f0c3789998417b728a4 on dugenkui03:patch-14** into **47561bdd855ab381113d736b5c26a09c08da590b on apache:develop**.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] MatrixHB commented on pull request #4138: `RemotingCommand` is not thread-safe
Posted by GitBox <gi...@apache.org>.
MatrixHB commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-1094278530
Why not use ConcurrentHashMap instead of HashMap？Using ConcurrentHashMap, no need to lock when reading, and the scope of locking is small when writing.
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dugenkui03 commented on pull request #4138: `RemotingCommand` is not thread-safe
Posted by GitBox <gi...@apache.org>.
dugenkui03 commented on PR #4138:
URL: https://github.com/apache/rocketmq/pull/4138#issuecomment-1094296953
> Why not use ConcurrentHashMap instead of HashMap？Using ConcurrentHashMap, no need to lock when reading, and the scope of locking is smaller when writing.
That's my opinion: Do as little as possible to fix mistakes.
# describe in Chinese
这是我做bug-fix的观点：做尽可能少的事情去修正错误。进行更进一步的优化则是另外一件事情。
我理解这里使用`synchronized`的原因是jvm对`synchronized`加锁进行了优化：如果加锁代码块没有真实的发生多线程并发访问，则永远不会真正的加锁。
--
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: dev-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] `RemotingCommand` is not thread-safe [rocketmq]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4138: `RemotingCommand` is not thread-safe
URL: https://github.com/apache/rocketmq/pull/4138
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org