[FLINK-12009] Fix wrong check message about heartbeat interval for HeartbeatServices #8048

yanghua · 2019-03-26T06:11:12Z

What is the purpose of the change

This pull request fixes wrong check message about heartbeat interval for HeartbeatServices

Brief change log

Fix wrong check message about heartbeat interval for HeartbeatServices

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (yes / no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
The serializers: (yes / no / don't know)
The runtime per-record code paths (performance sensitive): (yes / no / don't know)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / no / don't know)
The S3 file system connector: (yes / no / don't know)

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

flinkbot · 2019-03-26T06:11:47Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Review Progress

✅ 1. The [description] looks good.
- Approved by @sunjincheng121 [committer]
✅ 2. There is [consensus] that the contribution should go into to Flink.
- Approved by @sunjincheng121 [committer]
❓ 3. Needs [attention] from.
✅ 4. The change fits into the overall [architecture].
- Approved by @sunjincheng121 [committer]
✅ 5. Overall code [quality] is good.
- Approved by @sunjincheng121 [committer]

Please see the Pull Request Review Guide for a full explanation of the review process.

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

yanghua · 2019-03-27T05:44:48Z

@sunjincheng121 Can you review this PR?

lamberken · 2019-03-27T13:26:29Z

+1

azagrebin · 2019-04-02T09:01:19Z

flink-runtime/src/main/java/org/apache/flink/runtime/heartbeat/HeartbeatServices.java

@@ -40,7 +40,7 @@

 	public HeartbeatServices(long heartbeatInterval, long heartbeatTimeout) {
 		Preconditions.checkArgument(0L < heartbeatInterval, "The heartbeat interval must be larger than 0.");
-		Preconditions.checkArgument(heartbeatInterval <= heartbeatTimeout, "The heartbeat timeout should be larger or equal than the heartbeat timeout.");
+		Preconditions.checkArgument(heartbeatInterval <= heartbeatTimeout, "The heartbeat interval should be larger or equal than the heartbeat timeout.");


From the actual comparison, it looks like the second timeout should be changed to interval at the end of message.

The heartbeat interval should be larger or equal than the heartbeat timeout -> The heartbeat timeout should be larger or equal than the heartbeat interval

sunjincheng121

Good catch @yanghua!

Will be merged after you fix the comment.

Best,
Jincheng

sunjincheng121 · 2019-04-02T09:08:52Z

flink-runtime/src/main/java/org/apache/flink/runtime/heartbeat/HeartbeatServices.java

@@ -40,7 +40,7 @@

 	public HeartbeatServices(long heartbeatInterval, long heartbeatTimeout) {
 		Preconditions.checkArgument(0L < heartbeatInterval, "The heartbeat interval must be larger than 0.");
-		Preconditions.checkArgument(heartbeatInterval <= heartbeatTimeout, "The heartbeat timeout should be larger or equal than the heartbeat timeout.");
+		Preconditions.checkArgument(heartbeatInterval <= heartbeatTimeout, "The heartbeat interval should be larger or equal than the heartbeat timeout.");


The heartbeat interval should be larger or equal than the heartbeat timeout -> The heartbeat timeout should be larger or equal than the heartbeat interval

sunjincheng121 · 2019-04-02T09:12:01Z

@flinkbot approve description
@flinkbot approve consensus

…artbeatServices

yanghua · 2019-04-09T02:28:21Z

@azagrebin @sunjincheng121 Thanks for pointing the mistake and sorry for the late reply, I attended the Flink Forward SF 2019 and just come back today. I have updated the PR.

sunjincheng121 · 2019-04-09T02:36:43Z

Thank you for participating in 2019 flink forward, I believe that there is a great gain. @yanghua!

Merging...

@flinkbot approve all
Best,
Jincheng

…artbeatServices This closes apache#8048.

…al for HeartbeatServices This closes #8048.

…al for HeartbeatServices This closes apache#8048.

rmetzger added review=description? component=Runtime/Coordination labels Mar 26, 2019

azagrebin reviewed Apr 2, 2019

View reviewed changes

sunjincheng121 reviewed Apr 2, 2019

View reviewed changes

rmetzger added review=architecture? and removed review=description? labels Apr 2, 2019

[FLINK-12009] Fix wrong check message about heartbeat interval for He…

1ec4016

…artbeatServices

yanghua force-pushed the FLINK-12009 branch from c593078 to 1ec4016 Compare April 9, 2019 02:25

rmetzger added review=approved ✅ and removed review=architecture? labels Apr 9, 2019

sunjincheng121 pushed a commit to sunjincheng121/flink that referenced this pull request Apr 9, 2019

[FLINK-12009] Fix wrong check message about heartbeat interval for He…

1f1c0b5

…artbeatServices This closes apache#8048.

asfgit closed this in dca64c2 Apr 9, 2019

asfgit pushed a commit that referenced this pull request Apr 9, 2019

[FLINK-12009][runtime] Fix wrong check message about heartbeat interv…

03d0a39

…al for HeartbeatServices This closes #8048.

asfgit pushed a commit that referenced this pull request Apr 9, 2019

[FLINK-12009][runtime] Fix wrong check message about heartbeat interv…

5c4ed0e

…al for HeartbeatServices This closes #8048.

HuangZhenQiu pushed a commit to HuangZhenQiu/flink that referenced this pull request Apr 22, 2019

[FLINK-12009][runtime] Fix wrong check message about heartbeat interv…

2330088

…al for HeartbeatServices This closes apache#8048.

sunhaibotb pushed a commit to sunhaibotb/flink that referenced this pull request May 8, 2019

[FLINK-12009][runtime] Fix wrong check message about heartbeat interv…

3e31bfb

…al for HeartbeatServices This closes apache#8048.

tianchen92 pushed a commit to tianchen92/flink that referenced this pull request May 13, 2019

[FLINK-12009][runtime] Fix wrong check message about heartbeat interv…

7283ca1

…al for HeartbeatServices This closes apache#8048.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLINK-12009] Fix wrong check message about heartbeat interval for HeartbeatServices #8048

[FLINK-12009] Fix wrong check message about heartbeat interval for HeartbeatServices #8048

yanghua commented Mar 26, 2019

flinkbot commented Mar 26, 2019 •

edited

Loading

yanghua commented Mar 27, 2019

lamberken commented Mar 27, 2019

azagrebin Apr 2, 2019

sunjincheng121 Apr 2, 2019

sunjincheng121 left a comment •

edited

Loading

sunjincheng121 Apr 2, 2019

sunjincheng121 commented Apr 2, 2019

yanghua commented Apr 9, 2019

sunjincheng121 commented Apr 9, 2019

[FLINK-12009] Fix wrong check message about heartbeat interval for HeartbeatServices #8048

[FLINK-12009] Fix wrong check message about heartbeat interval for HeartbeatServices #8048

Conversation

yanghua commented Mar 26, 2019

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

flinkbot commented Mar 26, 2019 • edited Loading

Review Progress

yanghua commented Mar 27, 2019

lamberken commented Mar 27, 2019

azagrebin Apr 2, 2019

Choose a reason for hiding this comment

sunjincheng121 Apr 2, 2019

Choose a reason for hiding this comment

sunjincheng121 left a comment • edited Loading

Choose a reason for hiding this comment

sunjincheng121 Apr 2, 2019

Choose a reason for hiding this comment

sunjincheng121 commented Apr 2, 2019

yanghua commented Apr 9, 2019

sunjincheng121 commented Apr 9, 2019

flinkbot commented Mar 26, 2019 •

edited

Loading

sunjincheng121 left a comment •

edited

Loading