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

[Hotfix] Fix NettyRemotingClient might throw IllegalMonitorStateException #16038

Merged
merged 1 commit into from
May 22, 2024

Conversation

ruanwenjun
Copy link
Member

@ruanwenjun ruanwenjun commented May 20, 2024

Purpose of the pull request

Since future.await will release the monitor, if others want to notify the wait thread but doesn't get the log, then will throw IllegalMonitorStateException.

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@ruanwenjun ruanwenjun added bug Something isn't working 3.2.2 labels May 20, 2024
@ruanwenjun ruanwenjun added this to the 3.2.2 milestone May 20, 2024
try {
ChannelFuture future;
synchronized (bootstrap) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove synchronized here?

Copy link
Member Author

@ruanwenjun ruanwenjun May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to lock when we want to create a channel, Bootstrap is designed as threadsafe.

@codecov-commenter
Copy link

codecov-commenter commented May 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.60%. Comparing base (0fc2fe9) to head (e6211a8).

Current head e6211a8 differs from pull request most recent head 36652c2

Please upload reports for the commit 36652c2 to get more accurate results.

Additional details and impacted files
@@             Coverage Diff              @@
##                dev   #16038      +/-   ##
============================================
- Coverage     40.61%   40.60%   -0.01%     
+ Complexity     5221     5220       -1     
============================================
  Files          1381     1381              
  Lines         46080    46078       -2     
  Branches       4917     4917              
============================================
- Hits          18716    18711       -5     
- Misses        25439    25442       +3     
  Partials       1925     1925              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ruanwenjun ruanwenjun self-assigned this May 20, 2024
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hotfixNettyClient branch 3 times, most recently from 1a9dcb8 to a79ca2d Compare May 21, 2024 13:56
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hotfixNettyClient branch from a79ca2d to 36652c2 Compare May 21, 2024 13:58
Copy link
Member

@Radeity Radeity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

sonarcloud bot commented May 21, 2024

Copy link
Contributor

@rickchengx rickchengx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rickchengx rickchengx merged commit 56ecec7 into apache:dev May 22, 2024
63 checks passed
wangxj3 added a commit that referenced this pull request May 25, 2024
…ASF release (#16042)

* add version of 3.2.2

* Fix rpc channel leak due to concurrent operation (#16021)

* Fix rpc channel leak due to concurrent operation

* Throw channel create failed exception

---------

Co-authored-by: Rick Cheng <rickchengx@gmail.com>

* Fix WorkerTaskExecutorThreadPool#isOverload is incorrect (#16027)

* cp: Reduce the size of tarball to continue ASF release (#15004) (#15540)

* Reduce the size of tarball to continue ASF release

for more detail you can see https://lists.apache.org/thread/rmp7fghlj0n7h9y2v3p8gkw9f9qbo6qt
# Conflicts:
#	tools/dependencies/known-dependencies.txt

* Increase block-until-connected in ZookeeperRegistryTestCase (#16041)

* [Fix][CI] fix the ci error of Values.datasource.profile (#16031)

Co-authored-by: Eric Gao <ericgao.apache@gmail.com>

* Correction of Typos in the Chinese Document Appendix for Task Parameters (#16033)

Co-authored-by: Rick Cheng <rickchengx@gmail.com>

* [DSIP-39][parameter] Improvement startup parameters/global parameters/project parameters data type (#15967)

* [Improvement][parameter] New data types and type filtering

* [Improvement][parameter] Improvement startup parameters/global parameters data type

* fix api interfaces compatible

* add project parameter data type default value

* [Improvement][parameter] New data types and type filtering

* [Improvement][parameter] Improvement startup parameters/global parameters data type

* fix api interfaces compatible

* add project parameter data type default value

* improvement project code

* remove useless imports

* remove method onClearSearchTaskType

* add parameter doc

* optimisation logic

* code conflict resolution

* code conflict resolution

* [Improvement][Monitor] Show master && worker Busy Or Normal Status and Show Commands table list (#15978)

* update

* test

* add monitor enhance ui

* update

* update

* update doc

* fix spotless

* update

* update

* Update dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/DataAnalysisController.java

Co-authored-by: Wenjun Ruan <wenjun@apache.org>

* Update dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/DataAnalysisController.java

Co-authored-by: Wenjun Ruan <wenjun@apache.org>

* Update dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ErrorCommandMapper.java

Co-authored-by: Wenjun Ruan <wenjun@apache.org>

* Update dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ErrorCommandMapper.xml

Co-authored-by: Wenjun Ruan <wenjun@apache.org>

* Update dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/CommandMapper.java

Co-authored-by: Wenjun Ruan <wenjun@apache.org>

* Update dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ErrorCommandMapper.xml

Co-authored-by: Wenjun Ruan <wenjun@apache.org>

* update

* fix spotless

* update

---------

Co-authored-by: Wenjun Ruan <wenjun@apache.org>

* [Improvement][Monitor] Add UT for montor (#15998)

* formatting Code

* add pom

* Fix NettyRemotingClient might throw IllegalMonitorStateException (#16038)

* Removed unused StateEventHandleFailure (#16052)

* Remove OmitStackTraceInFastThrow in start.sh (#16054)

* Update Chart.yaml Rollback APPversion

* modify log msg (#16062)

Co-authored-by: xiangzihao <460888207@qq.com>

* [DS-16046][fix] Set PreparedStatement parameter type (#16050)

Fix the PreparedStatement parameter is TIME, set it to java.sql.Time

Co-authored-by: Aaron Wang <wangweirao16@gmail.com>
Co-authored-by: Rick Cheng <rickchengx@gmail.com>

* Update pom.xml

* [DSIP-42] Add dolphinscheduler-aws-authentication module  (#16043)

---------

Co-authored-by: wangxj <wangxj31>
Co-authored-by: Wenjun Ruan <wenjun@apache.org>
Co-authored-by: Rick Cheng <rickchengx@gmail.com>
Co-authored-by: Eric Gao <ericgao.apache@gmail.com>
Co-authored-by: TianXinCoord <tianxincoord@163.com>
Co-authored-by: 小可耐 <46134044+sdhzwc@users.noreply.github.com>
Co-authored-by: sleo <97011595+alei1206@users.noreply.github.com>
Co-authored-by: xiangzihao <460888207@qq.com>
Co-authored-by: yinxiaolog <43245392+yinxiaolog@users.noreply.github.com>
Co-authored-by: Aaron Wang <wangweirao16@gmail.com>
@ruanwenjun ruanwenjun deleted the dev_wenjun_hotfixNettyClient branch May 31, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.2.2 backend bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants