Re: [PR] [FLINK-35159] Transition ExecutionGraph to RUNNING after slot assignment [flink]
zentol merged PR #24680: URL: https://github.com/apache/flink/pull/24680 -- 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. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-35159] Transition ExecutionGraph to RUNNING after slot assignment [flink]
zentol commented on code in PR #24680: URL: https://github.com/apache/flink/pull/24680#discussion_r1571216654 ## flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/CreatingExecutionGraphTest.java: ## @@ -93,8 +93,12 @@ void testNotPossibleSlotAssignmentTransitionsToWaitingForResources() { ignored -> CreatingExecutionGraph.AssignmentResult.notPossible()); context.setExpectWaitingForResources(); +final StateTrackingMockExecutionGraph executionGraph = new StateTrackingMockExecutionGraph(); + executionGraphWithVertexParallelismFuture.complete( -getGraph(new StateTrackingMockExecutionGraph())); +getGraph(executionGraph)); + + assertThat(executionGraph.getState()).isEqualTo(JobStatus.INITIALIZING); Review Comment: > the graph would be running before the change yes -- 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. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-35159] Transition ExecutionGraph to RUNNING after slot assignment [flink]
zentol commented on code in PR #24680: URL: https://github.com/apache/flink/pull/24680#discussion_r1571218899 ## flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/CreatingExecutionGraphTest.java: ## @@ -166,18 +170,19 @@ static class MockCreatingExecutionGraphContext extends MockStateWithoutExecution new StateValidator<>("Executing"); private Function< - CreatingExecutionGraph.ExecutionGraphWithVertexParallelism, -CreatingExecutionGraph.AssignmentResult> +CreatingExecutionGraph.ExecutionGraphWithVertexParallelism, +CreatingExecutionGraph.AssignmentResult> tryToAssignSlotsFunction = -e -> CreatingExecutionGraph.AssignmentResult.success(e.getExecutionGraph()); +e -> CreatingExecutionGraph.AssignmentResult.success(e.getExecutionGraph()); private GlobalFailureHandler globalFailureHandler = t -> { // No-op. }; public void setExpectWaitingForResources() { -waitingForResourcesStateValidator.expectInput((none) -> {}); +waitingForResourcesStateValidator.expectInput((none) -> { Review Comment: No i screwed up :) My intellij is using a spotless config that isnt fully compatible with Flink... -- 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. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-35159] Transition ExecutionGraph to RUNNING after slot assignment [flink]
zentol commented on code in PR #24680: URL: https://github.com/apache/flink/pull/24680#discussion_r1571216390 ## flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/AdaptiveSchedulerTest.java: ## @@ -1783,56 +1778,6 @@ void testStopWithSavepointFailsInIllegalState() throws Exception { .withCauseInstanceOf(CheckpointException.class); } -@Test -void testSavepointFailsWhenBlockingEdgeExists() throws Exception { Review Comment: This was added in FLINK-34371 and is covered by tests in the DefaultScheduler: https://github.com/apache/flink/commit/d4e0084649c019c536ee1e44bab15c8eca01bf13#diff-b4bc1cd606feb86850a18371520b5dd63d02090b8567fdf80730d1d7dd6e693d -- 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. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-35159] Transition ExecutionGraph to RUNNING after slot assignment [flink]
dmvk commented on code in PR #24680: URL: https://github.com/apache/flink/pull/24680#discussion_r1571177099 ## flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/CreatingExecutionGraphTest.java: ## @@ -166,18 +170,19 @@ static class MockCreatingExecutionGraphContext extends MockStateWithoutExecution new StateValidator<>("Executing"); private Function< - CreatingExecutionGraph.ExecutionGraphWithVertexParallelism, -CreatingExecutionGraph.AssignmentResult> +CreatingExecutionGraph.ExecutionGraphWithVertexParallelism, +CreatingExecutionGraph.AssignmentResult> tryToAssignSlotsFunction = -e -> CreatingExecutionGraph.AssignmentResult.success(e.getExecutionGraph()); +e -> CreatingExecutionGraph.AssignmentResult.success(e.getExecutionGraph()); private GlobalFailureHandler globalFailureHandler = t -> { // No-op. }; public void setExpectWaitingForResources() { -waitingForResourcesStateValidator.expectInput((none) -> {}); +waitingForResourcesStateValidator.expectInput((none) -> { Review Comment: uuh, what's happening here 🤔 did we update formatter? ## flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/AdaptiveSchedulerTest.java: ## @@ -1783,56 +1778,6 @@ void testStopWithSavepointFailsInIllegalState() throws Exception { .withCauseInstanceOf(CheckpointException.class); } -@Test -void testSavepointFailsWhenBlockingEdgeExists() throws Exception { Review Comment: do you know why / when this was introduced? it indeed doesn't seem to be related AS; is this tested somewhere else? ## flink-runtime/src/test/java/org/apache/flink/runtime/scheduler/adaptive/CreatingExecutionGraphTest.java: ## @@ -93,8 +93,12 @@ void testNotPossibleSlotAssignmentTransitionsToWaitingForResources() { ignored -> CreatingExecutionGraph.AssignmentResult.notPossible()); context.setExpectWaitingForResources(); +final StateTrackingMockExecutionGraph executionGraph = new StateTrackingMockExecutionGraph(); + executionGraphWithVertexParallelismFuture.complete( -getGraph(new StateTrackingMockExecutionGraph())); +getGraph(executionGraph)); + + assertThat(executionGraph.getState()).isEqualTo(JobStatus.INITIALIZING); Review Comment: iiuc, the graph would be running before the change; makes sense -- 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. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org