You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/12/18 16:16:47 UTC
[GitHub] [flink] tillrohrmann opened a new pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
tillrohrmann opened a new pull request #14431:
URL: https://github.com/apache/flink/pull/14431
## What is the purpose of the change
This PR changes how the `JobMaster` is used by the `JobManagerRunnerImpl`. Instead of reusing the `JobMaster` across different leader sessions, the `JobManagerRunnerImpl` will create a new instance for every leader session. This makes the state management in the `JobMaster` easier because we don't have to make sure that the components are always in a cleaned up state when starting a new leader session. Moreover, it simplifies the state management because there are fewer mutable components in the `JobMaster`.
This PR is based on #14430.
## Brief change log
- 8faba3b: This commit changes how the JobManagerRunnerImpl uses JobMasterServices.
Now we use a JobMasterService per leader session.
- 436aa53: This commit changes the JobMaster to have a permanent fencing token.
- 1ab27e0: Since the JobMaster is now a PermanentlyFencedRpcEndpoint we no longer
need to make the scheduler resettable.
- 7b74054: Since we are no longer reusing the JobMaster across different leader sessions,
we can make the heartbeat managers final.
- aec1ac9: Make starting and stopping of JobMaster services symmetric
## Verifying this change
I adjusted several test to the new model. The test class `JobMasterTest` was mainly affected.
## 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, Kubernetes/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)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tillrohrmann commented on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748938515
@flinkbot run azure
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748184472
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.
## Automated Checks
Last check on commit aec1ac97effc6607d3fb4f94fe323c86c20759a8 (Fri Dec 18 16:19:47 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
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 <summary>Bot commands</summary>
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
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* aec1ac97effc6607d3fb4f94fe323c86c20759a8 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* aec1ac97effc6607d3fb4f94fe323c86c20759a8 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tillrohrmann closed pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
tillrohrmann closed pull request #14431:
URL: https://github.com/apache/flink/pull/14431
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tillrohrmann commented on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-749549484
Thanks for the review @zentol. Merging this PR now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"triggerType" : "PUSH"
}, {
"hash" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
} ]
}-->
## CI report:
* d7b68e6feb7c73809acfb52ac6007825d16accb5 Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117)
* a1183b9a28e2badd28b824e630c768c64cf2d01f UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "CANCELED",
"url" : "TBD",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"triggerType" : "PUSH"
}, {
"hash" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* Unknown: [CANCELED](TBD)
* d7b68e6feb7c73809acfb52ac6007825d16accb5 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117)
* a1183b9a28e2badd28b824e630c768c64cf2d01f UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tillrohrmann commented on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-749182205
AZP is now giving green light.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11122",
"triggerID" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d7b68e6feb7c73809acfb52ac6007825d16accb5 Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117)
* a1183b9a28e2badd28b824e630c768c64cf2d01f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11122)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tillrohrmann commented on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748930725
Thanks for the review @zentol. I have addressed your comments. Let's see what AZP now says about the tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11122",
"triggerID" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"triggerType" : "PUSH"
}, {
"hash" : "b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11128",
"triggerID" : "b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a1183b9a28e2badd28b824e630c768c64cf2d01f Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11122)
* b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11128)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tillrohrmann commented on a change in pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #14431:
URL: https://github.com/apache/flink/pull/14431#discussion_r547200872
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/JobMasterTest.java
##########
@@ -1452,20 +1345,17 @@ public JobGraph createKvJobGraph() {
public void testRequestKvStateWithIrrelevantRegistration() throws Exception {
final JobGraph graph = createKvJobGraph();
- final JobMaster jobMaster = createJobMaster(
- configuration,
- graph,
- haServices,
- new TestingJobManagerSharedServicesBuilder().build(),
- heartbeatServices);
+ final JobMaster jobMaster = new JobMasterBuilder(graph, rpcService)
+ .withConfiguration(configuration)
+ .withHighAvailabilityServices(haServices)
+ .withHeartbeatServices(heartbeatServices)
+ .createJobMaster();
+
+ jobMaster.start();
- CompletableFuture<Acknowledge> startFuture = jobMaster.start(jobMasterId);
final JobMasterGateway jobMasterGateway = jobMaster.getSelfGateway(JobMasterGateway.class);
try {
- // wait for the start to complete
- startFuture.get(testingTimeout.toMilliseconds(), TimeUnit.MILLISECONDS);
-
// register an irrelevant KvState
try {
jobMasterGateway.notifyKvStateRegistered(
Review comment:
There is no need for this. After `jobMaster.start()` is being called all subsequent messages which are sent to the `JobMaster` will be executed. The reason is that we only have a single mailbox which is processed one after another: `START; notifyKvStateRegistered; ....`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tillrohrmann commented on a change in pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #14431:
URL: https://github.com/apache/flink/pull/14431#discussion_r546659018
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/JobMasterSchedulerTest.java
##########
@@ -72,16 +70,13 @@ public void testIfStartSchedulingFailsJobMasterFailsFatally() throws Exception {
System.setSecurityManager(trackingSecurityManager);
final SchedulerNGFactory schedulerFactory = new FailingSchedulerFactory();
- final JobMaster jobMaster = new JobMasterBuilder(new JobGraph(), TESTING_RPC_SERVICE_RESOURCE
- .getTestingRpcService())
+ final JobMaster jobMaster = new JobMasterBuilder(new JobGraph(), TESTING_RPC_SERVICE_RESOURCE.getTestingRpcService())
.withSchedulerFactory(schedulerFactory)
.createJobMaster();
- final CompletableFuture<Acknowledge> startFuture = jobMaster.start(JobMasterId.generate());
+ jobMaster.start();
try {
- startFuture.join();
-
assertThat(trackingSecurityManager.getSystemExitFuture().join(), is(FatalExitExceptionHandler.EXIT_CODE));
Review comment:
The signal is now sent via the `OnCompletionActions` and no longer via the `SecurityManager`. I will update this test case.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] tillrohrmann commented on a change in pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
tillrohrmann commented on a change in pull request #14431:
URL: https://github.com/apache/flink/pull/14431#discussion_r546659238
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMaster.java
##########
@@ -797,52 +796,75 @@ private void startJobExecution() throws Exception {
}
private void startJobMasterServices() throws Exception {
- // start the slot pool make sure the slot pool now accepts messages for this leader
- slotPool.start(getFencingToken(), getAddress(), getMainThreadExecutor());
-
- //TODO: Remove once the ZooKeeperLeaderRetrieval returns the stored address upon start
- // try to reconnect to previously known leader
- reconnectToResourceManager(new FlinkException("Starting JobMaster component."));
-
- // job is ready to go, try to establish connection with resource manager
- // - activate leader retrieval for the resource manager
- // - on notification of the leader, the connection will be established and
- // the slot pool will start requesting slots
- resourceManagerLeaderRetriever.start(new ResourceManagerLeaderListener());
+ try {
+ // start the slot pool make sure the slot pool now accepts messages for this leader
+ slotPool.start(getFencingToken(), getAddress(), getMainThreadExecutor());
+
+ // job is ready to go, try to establish connection with resource manager
+ // - activate leader retrieval for the resource manager
+ // - on notification of the leader, the connection will be established and
+ // the slot pool will start requesting slots
+ resourceManagerLeaderRetriever.start(new ResourceManagerLeaderListener());
+ } catch (Exception e) {
+ handleStartJobMasterServicesError(e);
+ }
}
- /**
- * Suspending job, all the running tasks will be cancelled, and communication with other components
- * will be disposed.
- *
- * @param cause The reason of why this job been suspended.
- */
- private void suspendExecution(final Exception cause) {
- validateRunsInMainThread();
+ private void handleStartJobMasterServicesError(Exception e) throws Exception {
+ try {
+ stopJobMasterServices();
+ } catch (Exception inner) {
+ e.addSuppressed(inner);
+ }
+
+ throw e;
+ }
+
+ private void stopJobMasterServices() throws Exception {
+ Exception resultingException = null;
try {
resourceManagerLeaderRetriever.stop();
- resourceManagerAddress = null;
- } catch (Throwable t) {
- log.warn("Failed to stop resource manager leader retriever when suspending.", t);
+ } catch (Exception e) {
+ resultingException = e;
}
- suspendScheduler(cause);
+ // TODO: Distinguish between job termination which should free all slots and a loss of leadership which should keep the slots
+ slotPool.close();
+
+ stopHeartbeatServices();
+
+ ExceptionUtils.tryRethrowException(resultingException);
+ }
+
+ private void stopJobExecution(final Exception cause) throws Exception {
+ validateRunsInMainThread();
+
+ Exception resultingException = null;
Review comment:
yes will do.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11122",
"triggerID" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"triggerType" : "PUSH"
}, {
"hash" : "b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d7b68e6feb7c73809acfb52ac6007825d16accb5 Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117)
* a1183b9a28e2badd28b824e630c768c64cf2d01f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11122)
* b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zentol commented on a change in pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #14431:
URL: https://github.com/apache/flink/pull/14431#discussion_r546605468
##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobMaster.java
##########
@@ -797,52 +796,75 @@ private void startJobExecution() throws Exception {
}
private void startJobMasterServices() throws Exception {
- // start the slot pool make sure the slot pool now accepts messages for this leader
- slotPool.start(getFencingToken(), getAddress(), getMainThreadExecutor());
-
- //TODO: Remove once the ZooKeeperLeaderRetrieval returns the stored address upon start
- // try to reconnect to previously known leader
- reconnectToResourceManager(new FlinkException("Starting JobMaster component."));
-
- // job is ready to go, try to establish connection with resource manager
- // - activate leader retrieval for the resource manager
- // - on notification of the leader, the connection will be established and
- // the slot pool will start requesting slots
- resourceManagerLeaderRetriever.start(new ResourceManagerLeaderListener());
+ try {
+ // start the slot pool make sure the slot pool now accepts messages for this leader
+ slotPool.start(getFencingToken(), getAddress(), getMainThreadExecutor());
+
+ // job is ready to go, try to establish connection with resource manager
+ // - activate leader retrieval for the resource manager
+ // - on notification of the leader, the connection will be established and
+ // the slot pool will start requesting slots
+ resourceManagerLeaderRetriever.start(new ResourceManagerLeaderListener());
+ } catch (Exception e) {
+ handleStartJobMasterServicesError(e);
+ }
}
- /**
- * Suspending job, all the running tasks will be cancelled, and communication with other components
- * will be disposed.
- *
- * @param cause The reason of why this job been suspended.
- */
- private void suspendExecution(final Exception cause) {
- validateRunsInMainThread();
+ private void handleStartJobMasterServicesError(Exception e) throws Exception {
+ try {
+ stopJobMasterServices();
+ } catch (Exception inner) {
+ e.addSuppressed(inner);
+ }
+
+ throw e;
+ }
+
+ private void stopJobMasterServices() throws Exception {
+ Exception resultingException = null;
try {
resourceManagerLeaderRetriever.stop();
- resourceManagerAddress = null;
- } catch (Throwable t) {
- log.warn("Failed to stop resource manager leader retriever when suspending.", t);
+ } catch (Exception e) {
+ resultingException = e;
}
- suspendScheduler(cause);
+ // TODO: Distinguish between job termination which should free all slots and a loss of leadership which should keep the slots
+ slotPool.close();
+
+ stopHeartbeatServices();
+
+ ExceptionUtils.tryRethrowException(resultingException);
+ }
+
+ private void stopJobExecution(final Exception cause) throws Exception {
+ validateRunsInMainThread();
+
+ Exception resultingException = null;
Review comment:
Move this closer to the try block?
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/JobMasterSchedulerTest.java
##########
@@ -72,16 +70,13 @@ public void testIfStartSchedulingFailsJobMasterFailsFatally() throws Exception {
System.setSecurityManager(trackingSecurityManager);
final SchedulerNGFactory schedulerFactory = new FailingSchedulerFactory();
- final JobMaster jobMaster = new JobMasterBuilder(new JobGraph(), TESTING_RPC_SERVICE_RESOURCE
- .getTestingRpcService())
+ final JobMaster jobMaster = new JobMasterBuilder(new JobGraph(), TESTING_RPC_SERVICE_RESOURCE.getTestingRpcService())
.withSchedulerFactory(schedulerFactory)
.createJobMaster();
- final CompletableFuture<Acknowledge> startFuture = jobMaster.start(JobMasterId.generate());
+ jobMaster.start();
try {
- startFuture.join();
-
assertThat(trackingSecurityManager.getSystemExitFuture().join(), is(FatalExitExceptionHandler.EXIT_CODE));
Review comment:
This test got stuck on CI; I suppose we never call the fatal error handler since an error during the start of the scheduling now prevents the RpcEndpoint from starting altogether.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] zentol commented on a change in pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #14431:
URL: https://github.com/apache/flink/pull/14431#discussion_r547198838
##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/JobMasterTest.java
##########
@@ -1452,20 +1345,17 @@ public JobGraph createKvJobGraph() {
public void testRequestKvStateWithIrrelevantRegistration() throws Exception {
final JobGraph graph = createKvJobGraph();
- final JobMaster jobMaster = createJobMaster(
- configuration,
- graph,
- haServices,
- new TestingJobManagerSharedServicesBuilder().build(),
- heartbeatServices);
+ final JobMaster jobMaster = new JobMasterBuilder(graph, rpcService)
+ .withConfiguration(configuration)
+ .withHighAvailabilityServices(haServices)
+ .withHeartbeatServices(heartbeatServices)
+ .createJobMaster();
+
+ jobMaster.start();
- CompletableFuture<Acknowledge> startFuture = jobMaster.start(jobMasterId);
final JobMasterGateway jobMasterGateway = jobMaster.getSelfGateway(JobMasterGateway.class);
try {
- // wait for the start to complete
- startFuture.get(testingTimeout.toMilliseconds(), TimeUnit.MILLISECONDS);
-
// register an irrelevant KvState
try {
jobMasterGateway.notifyKvStateRegistered(
Review comment:
How do we guarantee that the JobMaster has already fully started at this point?
Until JobMaster#onStart has finished the AkkaRpcActor is still officially in a stopped state and would reject any incoming message, correct?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "",
"status" : "CANCELED",
"url" : "TBD",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* Unknown: [CANCELED](TBD)
* d7b68e6feb7c73809acfb52ac6007825d16accb5 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11122",
"triggerID" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"triggerType" : "PUSH"
}, {
"hash" : "b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11128",
"triggerID" : "b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11128)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* aec1ac97effc6607d3fb4f94fe323c86c20759a8 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #14431: [FLINK-11719] Do not reuse JobMaster instances across leader sessions
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #14431:
URL: https://github.com/apache/flink/pull/14431#issuecomment-748196435
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11050",
"triggerID" : "aec1ac97effc6607d3fb4f94fe323c86c20759a8",
"triggerType" : "PUSH"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117",
"triggerID" : "d7b68e6feb7c73809acfb52ac6007825d16accb5",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "748938515",
"triggerType" : "MANUAL"
}, {
"hash" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11122",
"triggerID" : "a1183b9a28e2badd28b824e630c768c64cf2d01f",
"triggerType" : "PUSH"
}, {
"hash" : "b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11128",
"triggerID" : "b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d7b68e6feb7c73809acfb52ac6007825d16accb5 Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11117)
* a1183b9a28e2badd28b824e630c768c64cf2d01f Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11122)
* b595aec4e7dacf3d70b4df2a96444bb3a0fbcae8 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11128)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org