Skip to content

[FIX_BUG_1.3#3789][remote]support netty heart beat#3868

Merged
davidzollo merged 9 commits intoapache:devfrom
CalvinKirs:netty_idle_heart
Oct 12, 2020
Merged

[FIX_BUG_1.3#3789][remote]support netty heart beat#3868
davidzollo merged 9 commits intoapache:devfrom
CalvinKirs:netty_idle_heart

Conversation

@CalvinKirs
Copy link
Copy Markdown
Member

@CalvinKirs CalvinKirs commented Oct 5, 2020

this closes #3789

@xingchun-chen
Copy link
Copy Markdown
Contributor

can you mention the code in 1.3.3-release

@xingchun-chen xingchun-chen added this to the 1.3.3-release milestone Oct 9, 2020
Copy link
Copy Markdown
Member

@gabry-lab gabry-lab left a comment

Choose a reason for hiding this comment

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

It's better if we distinguish the READER_IDLE and WRITER_IDLE event

@Override
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
if (evt instanceof IdleStateEvent) {
Command heartBeat = new Command();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's better if we distinguish the READER_IDLE and WRITER_IDLE event

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As far as heartbeat detection is concerned, this seems to be redundant. Regardless of the read and write events, they are all the same events. What do you think?

@Override
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
// send heartbeat when read idle.
if (evt instanceof IdleStateEvent) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's better if we distinguish the READER_IDLE and WRITER_IDLE event

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 10, 2020

Codecov Report

Merging #3868 into dev will increase coverage by 0.31%.
The diff coverage is 42.42%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #3868      +/-   ##
============================================
+ Coverage     40.04%   40.35%   +0.31%     
- Complexity     2925     2944      +19     
============================================
  Files           459      460       +1     
  Lines         21793    21817      +24     
  Branches       2652     2650       -2     
============================================
+ Hits           8726     8804      +78     
+ Misses        12238    12177      -61     
- Partials        829      836       +7     
Impacted Files Coverage Δ Complexity Δ
...e/dolphinscheduler/remote/command/CommandType.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
...pache/dolphinscheduler/remote/utils/Constants.java 83.33% <ø> (ø) 1.00 <0.00> (ø)
...inscheduler/remote/handler/NettyServerHandler.java 45.09% <20.00%> (-3.84%) 5.00 <1.00> (ø)
...inscheduler/remote/handler/NettyClientHandler.java 37.28% <26.66%> (-5.57%) 6.00 <2.00> (+1.00) ⬇️
...e/dolphinscheduler/remote/NettyRemotingClient.java 52.77% <100.00%> (ø) 11.00 <0.00> (ø)
...e/dolphinscheduler/remote/NettyRemotingServer.java 70.58% <100.00%> (+0.43%) 7.00 <1.00> (ø)
...inscheduler/service/zk/CuratorZookeeperClient.java 59.45% <0.00%> (-2.71%) 7.00% <0.00%> (ø%)
...nscheduler/api/service/impl/TenantServiceImpl.java 67.56% <0.00%> (-1.88%) 17.00% <0.00%> (ø%)
...api/service/impl/ProcessDefinitionServiceImpl.java 60.15% <0.00%> (-0.08%) 91.00% <0.00%> (ø%)
.../org/apache/dolphinscheduler/api/enums/Status.java 100.00% <0.00%> (ø) 5.00% <0.00%> (ø%)
... and 8 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 76f6e20...3801147. Read the comment docs.

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

62.2% 62.2% Coverage
0.0% 0.0% Duplication

@CalvinKirs
Copy link
Copy Markdown
Member Author

done

Copy link
Copy Markdown
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

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

+1
good job

@davidzollo davidzollo dismissed gabry-lab’s stale review October 12, 2020 15:29

no need to distinguish the read or write event

@davidzollo davidzollo merged commit 08dc691 into apache:dev Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug][remote] channel time out

5 participants