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/02/25 12:35:23 UTC

[GitHub] [flink] zhuzhurk opened a new pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

zhuzhurk opened a new pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213
 
 
   ## What is the purpose of the change
   
   Currently tests create DefaultScheduler via its constructor. Having a builder and a set of factory methods can significantly reduce the complexity to instantiate DefaultScheduler.
   It can be very helpful especially when we are to rework tests to base on the new scheduler.
   
   ## Brief change log
   
   Added a builder and factory methods to create DefaultScheduler for testing.
   
   ## Verifying this change
   
   This change is already covered by existing tests
   
   ## 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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-591980211
 
 
   Looks mostly good to me exception for one remark on the increase of the method visibility.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385128814
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   Can you tell me which test(s) you are planning to port to use the new scheduler next?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385532166
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   There would be quite a few tests to be reworked. 
   - ExecutionGraphDeploymentTest
   - ExecutionGraphPartitionReleaseTest
   - ExecutionGraphVariousFailuesTest
   - ExecutionTest
   - ExecutionVertexCancelTest
   - ExecutionVertexInputConstraintTest
   - ExecutionVertexTest
   - ExecutionGraphNotEnoughResourceTest
   - ExecutionGraphCheckpointCoordinatorTest
   - ExecutionGraphColocationRestartTest
   - ExecutionGraphSuspendTest
   - FinalizeOnMasterTest
   
   Most of them does not need much effort to be based on the new scheduler. Exposing ExecutionGraph would make the work much easier since we only need to force scheduling related actions to be conducted via the scheduler.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * cade94bacbaa7fa76c0a0d013eb069406d01b6ad Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/150784419) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384398068
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   It is not obvious to me why the visibility increase from `protected` to `public` is needed. If this is needed for a later change, then it should not be in this PR.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * dd6d5731a27212ea07f547e5653c07c7295d057d Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/150472562) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385534676
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   I will think again about whether we must expose `getExecutionGraph()` to the tests.
   Will drop this commit and may do it when reworking the tests. And as you mentioned, it should be a non-hotfix commit then if we would have it.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * dd6d5731a27212ea07f547e5653c07c7295d057d Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/150472562) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * dd6d5731a27212ea07f547e5653c07c7295d057d Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/150472562) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576) 
   * cade94bacbaa7fa76c0a0d013eb069406d01b6ad Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/150784419) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150968498",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "FAILURE",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/151047556",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * 3f9c1cdbb606073177669d281562ae2bdcd484e0 Travis: [FAILURE](https://travis-ci.com/flink-ci/flink/builds/151047556) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384455415
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   It will be needed by many tests to query the execution status when they are reworked to based on the new scheduler.
   It's a prerequisite change for a set of following PRs so I think we can avoid adding one more hotfix to make it public then.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384936512
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/DefaultScheduler.java
 ##########
 @@ -123,15 +121,15 @@ public DefaultScheduler(
 			backPressureStatsTracker,
 			ioExecutor,
 			jobMasterConfiguration,
-			slotProvider,
+			new ThrowingSlotProvider(), // this is not used any more in the new scheduler
 			futureExecutor,
 			userCodeLoader,
 			checkpointRecoveryFactory,
 			rpcTimeout,
 			new ThrowingRestartStrategy.ThrowingRestartStrategyFactory(),
 			blobWriter,
 			jobManagerJobMetricGroup,
-			slotRequestTimeout,
+			Time.seconds(0), // this is not used any more in the new scheduler
 
 Review comment:
   We can also remove these 2 params from the constructor of SchedulerBase once we have removed the legacy scheduler.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384935575
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,161 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
 
 Review comment:
   fixed

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot commented on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed 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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385667405
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,158 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
+	/**
+	 * Builder for {@link DefaultScheduler}.
+	 */
+	public static class DefaultSchedulerBuilder {
+		private final JobGraph jobGraph;
+
+		private SchedulingStrategyFactory schedulingStrategyFactory;
+
+		private Logger log = LOG;
+		private BackPressureStatsTracker backPressureStatsTracker = VoidBackPressureStatsTracker.INSTANCE;
+		private Executor ioExecutor = java.util.concurrent.Executors.newSingleThreadExecutor();
 
 Review comment:
   By merely creating a builder, we are instantiating several thread pools (`ioExecutor`, `futureExecutor`, `delayExecutor`) but from a user perspective there is no way to shut them down. I don't see a good solution to this.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385661925
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,158 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
+	/**
+	 * Builder for {@link DefaultScheduler}.
+	 */
+	public static class DefaultSchedulerBuilder {
+		private final JobGraph jobGraph;
+
+		private SchedulingStrategyFactory schedulingStrategyFactory;
+
+		private Logger log = LOG;
+		private BackPressureStatsTracker backPressureStatsTracker = VoidBackPressureStatsTracker.INSTANCE;
+		private Executor ioExecutor = java.util.concurrent.Executors.newSingleThreadExecutor();
+		private Configuration jobMasterConfiguration = new Configuration();
+		private ScheduledExecutorService futureExecutor = new DirectScheduledExecutorService();
+		private ScheduledExecutor delayExecutor = new ScheduledExecutorServiceAdapter(futureExecutor);
+		private ClassLoader userCodeLoader = getClass().getClassLoader();
 
 Review comment:
   `.getClassLoader()` can return `null`. I don't think this will happen in this code path but I would feel better if we used `ClassLoader.getSystemClassLoader()`

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385661567
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,158 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
+	/**
+	 * Builder for {@link DefaultScheduler}.
+	 */
+	public static class DefaultSchedulerBuilder {
+		private final JobGraph jobGraph;
+
+		private SchedulingStrategyFactory schedulingStrategyFactory;
+
+		private Logger log = LOG;
+		private BackPressureStatsTracker backPressureStatsTracker = VoidBackPressureStatsTracker.INSTANCE;
+		private Executor ioExecutor = java.util.concurrent.Executors.newSingleThreadExecutor();
+		private Configuration jobMasterConfiguration = new Configuration();
+		private ScheduledExecutorService futureExecutor = new DirectScheduledExecutorService();
+		private ScheduledExecutor delayExecutor = new ScheduledExecutorServiceAdapter(futureExecutor);
+		private ClassLoader userCodeLoader = getClass().getClassLoader();
 
 Review comment:
   `.getClassLoader()` can return `null`. I don't think this will happen in this code path but I would feel better if we used `ClassLoader.getSystemClassLoader()`

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384459803
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -77,6 +94,47 @@
 
 	private static final long DEFAULT_CHECKPOINT_TIMEOUT_MS = 10 * 60 * 1000;
 
+	private static final Time DEFAULT_TIMEOUT = Time.seconds(300);
+
+	private SchedulerTestingUtils() {}
+
+	public static DefaultSchedulerBuilder newSchedulerBuilder(final JobGraph jobGraph) {
+		return new DefaultSchedulerBuilder(jobGraph);
+	}
+
+	public static DefaultSchedulerBuilder newSchedulerBuilderWithDefaultSlotAllocator(
 
 Review comment:
   It will be used when we change some tests to create DefaultScheduler, one example can be found in `DefaultSchedulerBatchSchedulingTest#createScheduler()` in #10858 .
   Another example is `ExecutionGraphSuspendTest` which currently create ExecutionGraph and with a given slotProvider.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * dd6d5731a27212ea07f547e5653c07c7295d057d 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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590846030
 
 
   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 3f9c1cdbb606073177669d281562ae2bdcd484e0 (Fri Feb 28 21:50:08 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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150968498",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "FAILURE",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/151047556",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/151047556",
       "triggerID" : "592837488",
       "triggerType" : "MANUAL"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750",
       "triggerID" : "592837488",
       "triggerType" : "MANUAL"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * 3f9c1cdbb606073177669d281562ae2bdcd484e0 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/151047556) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150968498",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/151047556",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * 3f9c1cdbb606073177669d281562ae2bdcd484e0 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/151047556) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150968498",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "FAILURE",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/151047556",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/151047556",
       "triggerID" : "592837488",
       "triggerType" : "MANUAL"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750",
       "triggerID" : "592837488",
       "triggerType" : "MANUAL"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * 3f9c1cdbb606073177669d281562ae2bdcd484e0 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/151047556) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384456721
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,161 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
 
 Review comment:
   Agreed. Just did it by mistake.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385128814
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   Can you tell me which test(s) you are planning to port next?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * cade94bacbaa7fa76c0a0d013eb069406d01b6ad Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/150784419) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150968498",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * d557df5126a0a4f2404ac08ccea25000e8495e25 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/150968498) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384421553
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,161 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
 
 Review comment:
   There are more than two empty lines. Since this is forbidden by checkstyle in production code, I also wouldn't do it in test code.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385532166
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   There would be quite a few tests to be reworked. 
   - ExecutionGraphDeploymentTest
   - ExecutionGraphPartitionReleaseTest
   - ExecutionGraphVariousFailuesTest
   - ExecutionTest
   - ExecutionVertexCancelTest
   - ExecutionVertexInputConstraintTest
   - ExecutionVertexTest
   - ExecutionGraphNotEnoughResourceTest
   - ExecutionGraphCheckpointCoordinatorTest
   - ExecutionGraphColocationRestartTest
   - ExecutionGraphSuspendTest
   - FinalizeOnMasterTest
   
   Most of them does not need much effort to be based on the new scheduler if we exposes ExecutionGraph. It would make the work much easier since we only need to force scheduling related actions to be conducted via the scheduler.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150968498",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "FAILURE",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/151047556",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/151047556",
       "triggerID" : "592837488",
       "triggerType" : "MANUAL"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * 3f9c1cdbb606073177669d281562ae2bdcd484e0 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/151047556) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150968498",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * d557df5126a0a4f2404ac08ccea25000e8495e25 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/150968498) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719) 
   * 3f9c1cdbb606073177669d281562ae2bdcd484e0 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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot commented on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590846030
 
 
   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 b31d7fab8da81827ca063075427a3737377bf0ed (Tue Feb 25 12:37:53 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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385135983
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   > It's a prerequisite change for a set of following PRs so I think we can avoid adding one more hotfix to make it public then. 
   
   If you think it makes your life easier, then we can leave this commit. However, I would prefer to change the visibility in non-hotfix commit when we rewrite the first test to use the new scheduler. In the git history it will be easier to see the justification. Moreover, if we delay rewritig the tests, the visibility will not be unnecessarily increased.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * cade94bacbaa7fa76c0a0d013eb069406d01b6ad Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/150784419) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667) 
   * d557df5126a0a4f2404ac08ccea25000e8495e25 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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385749491
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,158 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
+	/**
+	 * Builder for {@link DefaultScheduler}.
+	 */
+	public static class DefaultSchedulerBuilder {
+		private final JobGraph jobGraph;
+
+		private SchedulingStrategyFactory schedulingStrategyFactory;
+
+		private Logger log = LOG;
+		private BackPressureStatsTracker backPressureStatsTracker = VoidBackPressureStatsTracker.INSTANCE;
+		private Executor ioExecutor = java.util.concurrent.Executors.newSingleThreadExecutor();
 
 Review comment:
   Good catch!
   I think we can borrow the idea of `ExecutionGraphTestUtils#createSimpleTestGraph()` which uses `TestingUtils.defaultExecutor()` for the `ioExecutor` and `futureExecutor`. `delayedExecutor` could still be a `ScheduledExecutorServiceAdapter` which wraps the `futureExecutor` by default.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385748972
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,158 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
+	/**
+	 * Builder for {@link DefaultScheduler}.
+	 */
+	public static class DefaultSchedulerBuilder {
+		private final JobGraph jobGraph;
+
+		private SchedulingStrategyFactory schedulingStrategyFactory;
+
+		private Logger log = LOG;
+		private BackPressureStatsTracker backPressureStatsTracker = VoidBackPressureStatsTracker.INSTANCE;
+		private Executor ioExecutor = java.util.concurrent.Executors.newSingleThreadExecutor();
+		private Configuration jobMasterConfiguration = new Configuration();
+		private ScheduledExecutorService futureExecutor = new DirectScheduledExecutorService();
+		private ScheduledExecutor delayExecutor = new ScheduledExecutorServiceAdapter(futureExecutor);
+		private ClassLoader userCodeLoader = getClass().getClassLoader();
 
 Review comment:
   Ok.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385661567
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,158 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
+	/**
+	 * Builder for {@link DefaultScheduler}.
+	 */
+	public static class DefaultSchedulerBuilder {
+		private final JobGraph jobGraph;
+
+		private SchedulingStrategyFactory schedulingStrategyFactory;
+
+		private Logger log = LOG;
+		private BackPressureStatsTracker backPressureStatsTracker = VoidBackPressureStatsTracker.INSTANCE;
+		private Executor ioExecutor = java.util.concurrent.Executors.newSingleThreadExecutor();
+		private Configuration jobMasterConfiguration = new Configuration();
+		private ScheduledExecutorService futureExecutor = new DirectScheduledExecutorService();
+		private ScheduledExecutor delayExecutor = new ScheduledExecutorServiceAdapter(futureExecutor);
+		private ClassLoader userCodeLoader = getClass().getClassLoader();
 
 Review comment:
   `.getClassLoader()` can return `null`. I don't think this will happen in this code path but I would feel better if we used `ClassLoader.getSystemClassLoader()`

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150968498",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * cade94bacbaa7fa76c0a0d013eb069406d01b6ad Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/150784419) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667) 
   * d557df5126a0a4f2404ac08ccea25000e8495e25 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/150968498) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385534676
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   I will think again about whether we must expose `getExecutionGraph()` to the tests.
   Will drop this commit and may do it when reworking 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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r385132307
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/SchedulerBase.java
 ##########
 @@ -280,23 +281,8 @@ private ExecutionGraph createExecutionGraph(
 			failoverStrategy);
 	}
 
-	/**
-	 * @deprecated Direct access to the execution graph by scheduler implementations is discouraged
-	 * because currently the execution graph has various features and responsibilities that a
-	 * scheduler should not be concerned about. The following specialized abstractions to the
-	 * execution graph and accessors should be preferred over direct access:
-	 * <ul>
-	 *     <li>{@link #getSchedulingTopology()}
-	 *     <li>{@link #getFailoverTopology()}
-	 *     <li>{@link #getInputsLocationsRetriever()}
-	 *     <li>{@link #getExecutionVertex(ExecutionVertexID)}
-	 *     <li>{@link #getExecutionVertexId(ExecutionAttemptID)}
-	 *     <li>{@link #getExecutionVertexIdOrThrow(ExecutionAttemptID)}
-	 * </ul>
-	 * Currently, only {@link LegacyScheduler} requires direct access to the execution graph.
-	 */
-	@Deprecated
-	protected ExecutionGraph getExecutionGraph() {
+	@VisibleForTesting
+	public ExecutionGraph getExecutionGraph() {
 
 Review comment:
   Regardless, I don't think we should remove the comment entirely. There is  a reason why helper methods such as `getExecutionVertex()` were introduced. There is business logic inside the `ExecutionGraph` and I would not want anyone to call it directly from the scheduler. We can add `@VisibleForTesting` but for me it does not necessarily mean that calling the method from production code is discouraged.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384407096
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,161 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
+
+	/**
+	 * Builder for {@link DefaultScheduler}.
+	 */
+	public static class DefaultSchedulerBuilder {
+		private final JobGraph jobGraph;
+
+		private SchedulingStrategyFactory schedulingStrategyFactory;
+
+		private Logger log = LOG;
+		private BackPressureStatsTracker backPressureStatsTracker = VoidBackPressureStatsTracker.INSTANCE;
+		private Executor ioExecutor = java.util.concurrent.Executors.newSingleThreadExecutor();
+		private Configuration jobMasterConfiguration = new Configuration();
+		private ScheduledExecutorService futureExecutor = new DirectScheduledExecutorService();
+		private ScheduledExecutor delayExecutor = new ScheduledExecutorServiceAdapter(futureExecutor);
+		private ClassLoader userCodeLoader = getClass().getClassLoader();
+		private CheckpointRecoveryFactory checkpointRecoveryFactory = new StandaloneCheckpointRecoveryFactory();
+		private Time rpcTimeout = DEFAULT_TIMEOUT;
+		private BlobWriter blobWriter = VoidBlobWriter.getInstance();
+		private JobManagerJobMetricGroup jobManagerJobMetricGroup = UnregisteredMetricGroups.createUnregisteredJobManagerJobMetricGroup();
+		private ShuffleMaster<?> shuffleMaster = NettyShuffleMaster.INSTANCE;
+		private JobMasterPartitionTracker partitionTracker = NoOpJobMasterPartitionTracker.INSTANCE;
+		private FailoverStrategy.Factory failoverStrategyFactory = new RestartPipelinedRegionFailoverStrategy.Factory();
+		private RestartBackoffTimeStrategy restartBackoffTimeStrategy = NoRestartBackoffTimeStrategy.INSTANCE;
+		private ExecutionVertexOperations executionVertexOperations = new DefaultExecutionVertexOperations();
+		private ExecutionVertexVersioner executionVertexVersioner = new ExecutionVertexVersioner();
+		private ExecutionSlotAllocatorFactory executionSlotAllocatorFactory = new TestExecutionSlotAllocatorFactory();
+
+		private DefaultSchedulerBuilder(final JobGraph jobGraph) {
+			this.jobGraph = jobGraph;
+
+			// scheduling strategy is by default set according to the scheduleMode. It can be re-assigned later.
+			this.schedulingStrategyFactory = DefaultSchedulerFactory.createSchedulingStrategyFactory(jobGraph.getScheduleMode());
+		}
+
+		public DefaultSchedulerBuilder setLogger(final Logger log) {
+			this.log = log;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setBackPressureStatsTracker(final BackPressureStatsTracker backPressureStatsTracker) {
+			this.backPressureStatsTracker = backPressureStatsTracker;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setIoExecutor(final Executor ioExecutor) {
+			this.ioExecutor = ioExecutor;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setJobMasterConfiguration(final Configuration jobMasterConfiguration) {
+			this.jobMasterConfiguration = jobMasterConfiguration;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setFutureExecutor(final ScheduledExecutorService futureExecutor) {
+			this.futureExecutor = futureExecutor;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setDelayExecutor(final ScheduledExecutor delayExecutor) {
+			this.delayExecutor = delayExecutor;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setUserCodeLoader(final ClassLoader userCodeLoader) {
+			this.userCodeLoader = userCodeLoader;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setCheckpointRecoveryFactory(final CheckpointRecoveryFactory checkpointRecoveryFactory) {
+			this.checkpointRecoveryFactory = checkpointRecoveryFactory;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setRpcTimeout(final Time rpcTimeout) {
+			this.rpcTimeout = rpcTimeout;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setBlobWriter(final BlobWriter blobWriter) {
+			this.blobWriter = blobWriter;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setJobManagerJobMetricGroup(final JobManagerJobMetricGroup jobManagerJobMetricGroup) {
+			this.jobManagerJobMetricGroup = jobManagerJobMetricGroup;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setShuffleMaster(final ShuffleMaster<?> shuffleMaster) {
+			this.shuffleMaster = shuffleMaster;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setPartitionTracker(final JobMasterPartitionTracker partitionTracker) {
+			this.partitionTracker = partitionTracker;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setSchedulingStrategyFactory(final SchedulingStrategyFactory schedulingStrategyFactory) {
+			this.schedulingStrategyFactory = schedulingStrategyFactory;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setFailoverStrategyFactory(final FailoverStrategy.Factory failoverStrategyFactory) {
+			this.failoverStrategyFactory = failoverStrategyFactory;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setRestartBackoffTimeStrategy(final RestartBackoffTimeStrategy restartBackoffTimeStrategy) {
+			this.restartBackoffTimeStrategy = restartBackoffTimeStrategy;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setExecutionVertexOperations(final ExecutionVertexOperations executionVertexOperations) {
+			this.executionVertexOperations = executionVertexOperations;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setExecutionVertexVersioner(final ExecutionVertexVersioner executionVertexVersioner) {
+			this.executionVertexVersioner = executionVertexVersioner;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setExecutionSlotAllocatorFactory(final ExecutionSlotAllocatorFactory executionSlotAllocatorFactory) {
+			this.executionSlotAllocatorFactory = executionSlotAllocatorFactory;
+			return this;
+		}
+
+		public DefaultScheduler build() throws Exception {
+			return new DefaultScheduler(
+				log,
+				jobGraph,
+				backPressureStatsTracker,
+				ioExecutor,
+				jobMasterConfiguration,
+				new SimpleSlotProvider(jobGraph.getJobID(), 0), // this is not used any more in the new scheduler
 
 Review comment:
   I don't think the comment `// this is not used any more in the new scheduler` is needed. It would not be consistent to comment here because there is another invocation of this constructor where we do not comment. I would remove the parameters `slotProvider` and `slotRequestTimeout` from the `DefaultScheduler`, and if possible pass a _"throwing"_ implementation/invalid value to `SchedulerBase`.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150784419",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5667",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150968498",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719",
       "triggerID" : "d557df5126a0a4f2404ac08ccea25000e8495e25",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     }, {
       "hash" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/151047556",
       "triggerID" : "3f9c1cdbb606073177669d281562ae2bdcd484e0",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * d557df5126a0a4f2404ac08ccea25000e8495e25 Travis: [SUCCESS](https://travis-ci.com/flink-ci/flink/builds/150968498) Azure: [FAILURE](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5719) 
   * 3f9c1cdbb606073177669d281562ae2bdcd484e0 Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/151047556) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5750) 
   
   <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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384935971
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -213,4 +267,161 @@ public static CheckpointCoordinator getCheckpointCoordinator(SchedulerBase sched
 			return operatorGateway.sendOperatorEventToTask(task, operator, evt);
 		}
 	}
+
+
+	/**
+	 * Builder for {@link DefaultScheduler}.
+	 */
+	public static class DefaultSchedulerBuilder {
+		private final JobGraph jobGraph;
+
+		private SchedulingStrategyFactory schedulingStrategyFactory;
+
+		private Logger log = LOG;
+		private BackPressureStatsTracker backPressureStatsTracker = VoidBackPressureStatsTracker.INSTANCE;
+		private Executor ioExecutor = java.util.concurrent.Executors.newSingleThreadExecutor();
+		private Configuration jobMasterConfiguration = new Configuration();
+		private ScheduledExecutorService futureExecutor = new DirectScheduledExecutorService();
+		private ScheduledExecutor delayExecutor = new ScheduledExecutorServiceAdapter(futureExecutor);
+		private ClassLoader userCodeLoader = getClass().getClassLoader();
+		private CheckpointRecoveryFactory checkpointRecoveryFactory = new StandaloneCheckpointRecoveryFactory();
+		private Time rpcTimeout = DEFAULT_TIMEOUT;
+		private BlobWriter blobWriter = VoidBlobWriter.getInstance();
+		private JobManagerJobMetricGroup jobManagerJobMetricGroup = UnregisteredMetricGroups.createUnregisteredJobManagerJobMetricGroup();
+		private ShuffleMaster<?> shuffleMaster = NettyShuffleMaster.INSTANCE;
+		private JobMasterPartitionTracker partitionTracker = NoOpJobMasterPartitionTracker.INSTANCE;
+		private FailoverStrategy.Factory failoverStrategyFactory = new RestartPipelinedRegionFailoverStrategy.Factory();
+		private RestartBackoffTimeStrategy restartBackoffTimeStrategy = NoRestartBackoffTimeStrategy.INSTANCE;
+		private ExecutionVertexOperations executionVertexOperations = new DefaultExecutionVertexOperations();
+		private ExecutionVertexVersioner executionVertexVersioner = new ExecutionVertexVersioner();
+		private ExecutionSlotAllocatorFactory executionSlotAllocatorFactory = new TestExecutionSlotAllocatorFactory();
+
+		private DefaultSchedulerBuilder(final JobGraph jobGraph) {
+			this.jobGraph = jobGraph;
+
+			// scheduling strategy is by default set according to the scheduleMode. It can be re-assigned later.
+			this.schedulingStrategyFactory = DefaultSchedulerFactory.createSchedulingStrategyFactory(jobGraph.getScheduleMode());
+		}
+
+		public DefaultSchedulerBuilder setLogger(final Logger log) {
+			this.log = log;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setBackPressureStatsTracker(final BackPressureStatsTracker backPressureStatsTracker) {
+			this.backPressureStatsTracker = backPressureStatsTracker;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setIoExecutor(final Executor ioExecutor) {
+			this.ioExecutor = ioExecutor;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setJobMasterConfiguration(final Configuration jobMasterConfiguration) {
+			this.jobMasterConfiguration = jobMasterConfiguration;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setFutureExecutor(final ScheduledExecutorService futureExecutor) {
+			this.futureExecutor = futureExecutor;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setDelayExecutor(final ScheduledExecutor delayExecutor) {
+			this.delayExecutor = delayExecutor;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setUserCodeLoader(final ClassLoader userCodeLoader) {
+			this.userCodeLoader = userCodeLoader;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setCheckpointRecoveryFactory(final CheckpointRecoveryFactory checkpointRecoveryFactory) {
+			this.checkpointRecoveryFactory = checkpointRecoveryFactory;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setRpcTimeout(final Time rpcTimeout) {
+			this.rpcTimeout = rpcTimeout;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setBlobWriter(final BlobWriter blobWriter) {
+			this.blobWriter = blobWriter;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setJobManagerJobMetricGroup(final JobManagerJobMetricGroup jobManagerJobMetricGroup) {
+			this.jobManagerJobMetricGroup = jobManagerJobMetricGroup;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setShuffleMaster(final ShuffleMaster<?> shuffleMaster) {
+			this.shuffleMaster = shuffleMaster;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setPartitionTracker(final JobMasterPartitionTracker partitionTracker) {
+			this.partitionTracker = partitionTracker;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setSchedulingStrategyFactory(final SchedulingStrategyFactory schedulingStrategyFactory) {
+			this.schedulingStrategyFactory = schedulingStrategyFactory;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setFailoverStrategyFactory(final FailoverStrategy.Factory failoverStrategyFactory) {
+			this.failoverStrategyFactory = failoverStrategyFactory;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setRestartBackoffTimeStrategy(final RestartBackoffTimeStrategy restartBackoffTimeStrategy) {
+			this.restartBackoffTimeStrategy = restartBackoffTimeStrategy;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setExecutionVertexOperations(final ExecutionVertexOperations executionVertexOperations) {
+			this.executionVertexOperations = executionVertexOperations;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setExecutionVertexVersioner(final ExecutionVertexVersioner executionVertexVersioner) {
+			this.executionVertexVersioner = executionVertexVersioner;
+			return this;
+		}
+
+		public DefaultSchedulerBuilder setExecutionSlotAllocatorFactory(final ExecutionSlotAllocatorFactory executionSlotAllocatorFactory) {
+			this.executionSlotAllocatorFactory = executionSlotAllocatorFactory;
+			return this;
+		}
+
+		public DefaultScheduler build() throws Exception {
+			return new DefaultScheduler(
+				log,
+				jobGraph,
+				backPressureStatsTracker,
+				ioExecutor,
+				jobMasterConfiguration,
+				new SimpleSlotProvider(jobGraph.getJobID(), 0), // this is not used any more in the new scheduler
 
 Review comment:
   Good suggestion. 1b292e3cce846b600e841ba0fdab9668031edfd0 is added to remove the these two params of DefaultScheduler.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk merged pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk merged pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
GJL commented on a change in pull request #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#discussion_r384422079
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/SchedulerTestingUtils.java
 ##########
 @@ -77,6 +94,47 @@
 
 	private static final long DEFAULT_CHECKPOINT_TIMEOUT_MS = 10 * 60 * 1000;
 
+	private static final Time DEFAULT_TIMEOUT = Time.seconds(300);
+
+	private SchedulerTestingUtils() {}
+
+	public static DefaultSchedulerBuilder newSchedulerBuilder(final JobGraph jobGraph) {
+		return new DefaultSchedulerBuilder(jobGraph);
+	}
+
+	public static DefaultSchedulerBuilder newSchedulerBuilderWithDefaultSlotAllocator(
 
 Review comment:
   I cannot find real usages of these `newSchedulerBuilderWithDefaultSlotAllocator()` and `createScheduler()`. Can you explain why they are needed?

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] zhuzhurk commented on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
zhuzhurk commented on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-592837488
 
 
   @flinkbot run travis

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11213: [FLINK-16276][tests] Introduce a builder and factory methods to create DefaultScheduler for testing
URL: https://github.com/apache/flink/pull/11213#issuecomment-590853060
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b31d7fab8da81827ca063075427a3737377bf0ed",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/flink-ci/flink/builds/150472562",
       "triggerID" : "dd6d5731a27212ea07f547e5653c07c7295d057d",
       "triggerType" : "PUSH"
     }, {
       "hash" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "cade94bacbaa7fa76c0a0d013eb069406d01b6ad",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * b31d7fab8da81827ca063075427a3737377bf0ed UNKNOWN
   * dd6d5731a27212ea07f547e5653c07c7295d057d Travis: [PENDING](https://travis-ci.com/flink-ci/flink/builds/150472562) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=5576) 
   * cade94bacbaa7fa76c0a0d013eb069406d01b6ad 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


With regards,
Apache Git Services