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 2019/01/28 14:53:59 UTC

[GitHub] tillrohrmann commented on a change in pull request #7564: [FLINK-11415] Introduce JobMasterServiceFactory

tillrohrmann commented on a change in pull request #7564: [FLINK-11415] Introduce JobMasterServiceFactory
URL: https://github.com/apache/flink/pull/7564#discussion_r251447222
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobManagerRunner.java
 ##########
 @@ -140,29 +132,10 @@ public JobManagerRunner(
 			this.runningJobsRegistry = haServices.getRunningJobsRegistry();
 			this.leaderElectionService = haServices.getJobManagerLeaderElectionService(jobGraph.getJobID());
 
-			final JobMasterConfiguration jobMasterConfiguration = JobMasterConfiguration.fromConfiguration(configuration);
-
 			this.leaderGatewayFuture = new CompletableFuture<>();
 
-			final SlotPoolFactory slotPoolFactory = DefaultSlotPoolFactory.fromConfiguration(
-				configuration,
-				rpcService);
-
 			// now start the JobManager
-			this.jobMasterService = new JobMaster(
-				rpcService,
-				jobMasterConfiguration,
-				resourceId,
-				jobGraph,
-				haServices,
-				slotPoolFactory,
-				jobManagerSharedServices,
-				heartbeatServices,
-				blobServer,
-				jobManagerJobMetricGroupFactory,
-				this,
-				fatalErrorHandler,
-				userCodeLoader);
+			this.jobMasterService = jobMasterFactory.createJobMasterService(jobGraph, this, userCodeLoader);
 
 Review comment:
   This would not be easily possible because you also want to pass in a `JobCompletionActions` which atm is implemented by the `JobManagerRunner`. Moreover one would separate the `libraryCacheManager.registerJob` from the unregistration in the `close` method of the `JobManagerRunner`. I think there is some value in keeping the setup and cleanup logic in one place. Another aspect is that in the future I would like to change the `JobManagerRunner` in such a way that it is only created after the `JobManagerRunner` has gained leadership. This will things much simpler wrt the lifecycle management of the `JobMaster`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services