[FLINK-5918] [runtime] port range support for config taskmanager.rpc.port #3416
Conversation
barcahead
force-pushed
the
FLINK-5918
branch
from
2f925fe
to
7cc2df3
Compare
flink-core/pom.xml
Outdated
| @@ -165,6 +165,7 @@ under the License. | |||
| <exclude>org.apache.flink.configuration.ConfigConstants#ENABLE_QUARANTINE_MONITOR</exclude> | |||
| <exclude>org.apache.flink.configuration.ConfigConstants#NETWORK_REQUEST_BACKOFF_INITIAL_KEY</exclude> | |||
| <exclude>org.apache.flink.configuration.ConfigConstants#NETWORK_REQUEST_BACKOFF_MAX_KEY</exclude> | |||
| <exclude>org.apache.flink.configuration.ConfigConstants#DEFAULT_TASK_MANAGER_IPC_PORT</exclude> | |||
There was a problem hiding this comment.
Adding an exclusion here should only be the last resort; converting the old constant to the new one is trivial.
There was a problem hiding this comment.
Thanks for the review, I will use a new constant instead.
There was a problem hiding this comment.
Changes look good to me except for the change of the config constants variable type as remarked by @zentol .
| fn: => T, | ||
| stopCond: => Boolean, | ||
| maxSleepBetweenRetries : Long = 0 ) | ||
| : scala.util.Try[T] = { |
There was a problem hiding this comment.
Flink's (implicit) Scala style is as follows:
def foobar(
a: Int,
b: Float)
: Double = {
// barfoo
}
There was a problem hiding this comment.
@tillrohrmann the style is fixed :-)
… port range instead
|
@zentol @tillrohrmann Thanks for your review, I have addressed your comments and updated the PR. |
|
merging. |
|
@zentol Has this been merged? |
|
no I haven't merged it yet. I wanted to try it out once more but forgot about it :( |
|
merging, sorry it took so long! |
add port range support for config
taskmanager.rpc.portusing existing helper functions. The port range format follows the convention ofblob.server.port.General
Documentation
Tests & Build
mvn clean verifyhas been executed successfully locally or a Travis build has passed