This is an automated email from the ASF dual-hosted git repository.

1996fanrui pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/flink.git


The following commit(s) were added to refs/heads/master by this push:
     new dd33da5a2da [FLINK-40520][checkpointing] Release spilled channel state 
on recovery abort between fetch and drain
dd33da5a2da is described below

commit dd33da5a2da017f1020258c9d73ca84b968c64d5
Author: Rui Fan <[email protected]>
AuthorDate: Wed Sep 9 23:48:34 2026 +0200

    [FLINK-40520][checkpointing] Release spilled channel state on recovery 
abort between fetch and drain
    
    Spill files produced by readInputData were only deleted once drain() ran, 
so any
    abort between fetch and drain leaked them until TaskManager shutdown.
    
    - readInputData deletes the handler's spill directory if it fails before 
handing the
      state off. This does not depend on the produced FetchedChannelState 
having been
      built, since stateHandler.close() itself may be what failed.
    - fetchChannelState registers the fetched state with the task's 
resourceCloser, so
      cleanUp() deletes the spill files whenever recovery aborts afterwards 
(drainer
      never built, mailbox mail rejected or dropped, drain() never scheduled). 
close()
      is idempotent, so a completed drain makes this a no-op; a fetch that 
finishes
      after cleanUp() is closed by the registry on the spot.
---
 .../channel/RecoveredChannelStateHandler.java      | 14 ++++
 .../channel/SequentialChannelStateReaderImpl.java  |  8 ++
 .../flink/streaming/runtime/tasks/StreamTask.java  |  6 ++
 .../SequentialChannelStateReaderImplTest.java      | 94 ++++++++++++++++++++++
 4 files changed, 122 insertions(+)

diff --git 
a/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/channel/RecoveredChannelStateHandler.java
 
b/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/channel/RecoveredChannelStateHandler.java
index 03ac7f8309c..427f1d5cc9e 100644
--- 
a/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/channel/RecoveredChannelStateHandler.java
+++ 
b/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/channel/RecoveredChannelStateHandler.java
@@ -36,6 +36,8 @@ import 
org.apache.flink.runtime.io.network.partition.CheckpointedResultPartition
 import org.apache.flink.runtime.io.network.partition.consumer.InputChannel;
 import org.apache.flink.runtime.io.network.partition.consumer.InputGate;
 import 
org.apache.flink.runtime.io.network.partition.consumer.RecoveredInputChannel;
+import org.apache.flink.util.FileUtils;
+import org.apache.flink.util.IOUtils;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -413,6 +415,18 @@ abstract class AbstractSpillingHandler extends 
AbstractInputChannelRecoveredStat
         return files;
     }
 
+    /**
+     * Deletes the spill directory with everything written so far, whether or 
not {@link
+     * #closeInternal()} has built {@link #producedChannelState}. Best-effort, 
never throws.
+     */
+    void discardSpilledFiles() {
+        IOUtils.closeQuietly(currentStream);
+        currentStream = null;
+        // Marks the state closed for anyone still holding it before the 
directory goes away.
+        IOUtils.closeQuietly(producedChannelState);
+        FileUtils.deleteDirectoryQuietly(baseDir.toFile());
+    }
+
     /**
      * Seals the open segment and the file stream, then builds the {@link 
FetchedChannelState}
      * handoff from the written files. Produces nothing if no bytes were ever 
spilled.
diff --git 
a/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/channel/SequentialChannelStateReaderImpl.java
 
b/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/channel/SequentialChannelStateReaderImpl.java
index 9284c226a59..9b35f1209c1 100644
--- 
a/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/channel/SequentialChannelStateReaderImpl.java
+++ 
b/flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/channel/SequentialChannelStateReaderImpl.java
@@ -103,6 +103,14 @@ public class SequentialChannelStateReaderImpl implements 
SequentialChannelStateR
             // stateHandler.close() (above) has flushed the filter writer and 
published the
             // produced spill file, so read getProducedChannelState() after 
the close completes.
             return Optional.ofNullable(stateHandler.getProducedChannelState());
+        } catch (Throwable t) {
+            // The state was not handed off, so no drainer will release its 
spill files: delete
+            // them here. This does not rely on the produced state having been 
built, since
+            // stateHandler.close() itself may be what failed.
+            if (stateHandler instanceof AbstractSpillingHandler) {
+                ((AbstractSpillingHandler) stateHandler).discardSpilledFiles();
+            }
+            throw t;
         }
     }
 
diff --git 
a/flink-runtime/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java
 
b/flink-runtime/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java
index 113f3ea046d..7bccacd79dd 100644
--- 
a/flink-runtime/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java
+++ 
b/flink-runtime/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java
@@ -1081,6 +1081,12 @@ public abstract class StreamTask<OUT, OP extends 
StreamOperator<OUT>>
             Optional<FetchedChannelState> state =
                     reader.readInputData(inputGates, 
createRecordFilterContext());
             if (state.isPresent()) {
+                // The task owns the spill files until a drainer releases 
them; registering here
+                // deletes them on cleanUp() if recovery aborts anywhere 
between fetch and drain
+                // (the drainer never gets built, a mailbox mail is rejected 
or dropped, drain() is
+                // never scheduled). close() is idempotent, so a completed 
drain makes this a no-op.
+                // If cleanUp() already ran, the registry closes the state 
right here instead.
+                resourceCloser.registerCloseable(state.get());
                 LOG.info(
                         "Fetched and filtered the recovered channel state into 
{} spill file(s).",
                         state.get().files().size());
diff --git 
a/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/channel/SequentialChannelStateReaderImplTest.java
 
b/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/channel/SequentialChannelStateReaderImplTest.java
index dc09e35cffc..a7c3b8b26e4 100644
--- 
a/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/channel/SequentialChannelStateReaderImplTest.java
+++ 
b/flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/channel/SequentialChannelStateReaderImplTest.java
@@ -18,8 +18,11 @@
 package org.apache.flink.runtime.checkpoint.channel;
 
 import org.apache.flink.api.java.tuple.Tuple2;
+import org.apache.flink.core.fs.FSDataInputStream;
 import org.apache.flink.core.memory.MemorySegmentFactory;
+import org.apache.flink.runtime.checkpoint.InflightDataRescalingDescriptor;
 import org.apache.flink.runtime.checkpoint.OperatorSubtaskState;
+import org.apache.flink.runtime.checkpoint.StateObjectCollection;
 import org.apache.flink.runtime.checkpoint.TaskStateSnapshot;
 import org.apache.flink.runtime.io.network.buffer.Buffer;
 import org.apache.flink.runtime.io.network.buffer.FreeingBufferRecycler;
@@ -40,6 +43,7 @@ import org.apache.flink.runtime.jobgraph.OperatorID;
 import org.apache.flink.runtime.state.InputChannelStateHandle;
 import org.apache.flink.runtime.state.ResultSubpartitionStateHandle;
 import org.apache.flink.runtime.state.memory.ByteStreamStateHandle;
+import org.apache.flink.runtime.state.testutils.EmptyStreamStateHandle;
 import org.apache.flink.streaming.runtime.io.recovery.RecordFilterContext;
 import org.apache.flink.testutils.junit.extensions.parameterized.Parameter;
 import 
org.apache.flink.testutils.junit.extensions.parameterized.ParameterizedTestExtension;
@@ -51,10 +55,13 @@ import 
org.apache.flink.shaded.guava33.com.google.common.io.Closer;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.TestTemplate;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
 
 import java.io.ByteArrayOutputStream;
 import java.io.DataOutputStream;
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -65,6 +72,7 @@ import java.util.Optional;
 import java.util.Random;
 import java.util.function.BiFunction;
 import java.util.function.Function;
+import java.util.stream.Stream;
 
 import static java.util.function.Function.identity;
 import static java.util.stream.Collectors.toList;
@@ -73,6 +81,8 @@ import static java.util.stream.IntStream.range;
 import static 
org.apache.flink.runtime.state.ChannelStateHelper.castToInputStateCollection;
 import static 
org.apache.flink.runtime.state.ChannelStateHelper.castToOutputStateCollection;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
 /** {@link SequentialChannelStateReaderImpl} Test. */
 @ExtendWith(ParameterizedTestExtension.class)
@@ -107,6 +117,8 @@ public class SequentialChannelStateReaderImplTest {
     @Parameter(value = 5)
     public int bufferSize;
 
+    @TempDir Path tempDir;
+
     private ChannelStateSerializer serializer;
     private Random random;
     private int buffersPerChannel;
@@ -156,6 +168,88 @@ public class SequentialChannelStateReaderImplTest {
                 });
     }
 
+    @TestTemplate
+    void testReadInputDataDeletesSpillFilesOnAbort() throws Exception {
+        // The empty-state parameter combos spill nothing, so there is nothing 
to assert; skip them.
+        assumeTrue(stateParLevel > 0 && parLevel > 0);
+
+        Map<InputChannelInfo, List<byte[]>> inputChannelsData =
+                generateState(InputChannelInfo::new);
+
+        // read#1 spills valid input-channel state; read#2's delegate fails, 
so readInputData aborts
+        // after spilling.
+        List<InputChannelStateHandle> validInputState =
+                writePermuted(inputChannelsData, Collections.emptyMap()).f0;
+        InputChannelStateHandle failingUpstreamState =
+                new InputChannelStateHandle(
+                        new InputChannelInfo(0, 0),
+                        new FailingStreamStateHandle(),
+                        Collections.singletonList(0L));
+        TaskStateSnapshot snapshot =
+                new TaskStateSnapshot(
+                        Collections.singletonMap(
+                                new OperatorID(),
+                                OperatorSubtaskState.builder()
+                                        .setInputChannelState(
+                                                
castToInputStateCollection(validInputState))
+                                        .setUpstreamOutputBufferState(
+                                                new StateObjectCollection<>(
+                                                        
Collections.singletonList(
+                                                                
failingUpstreamState)))
+                                        .build()));
+
+        SequentialChannelStateReader reader = new 
SequentialChannelStateReaderImpl(snapshot);
+        // Empty inputConfigs -> no filtering handler -> the plain 
SpillingNoFilteringHandler path.
+        RecordFilterContext spillingContext =
+                new RecordFilterContext(
+                        new RecordFilterContext.InputFilterConfig[0],
+                        InflightDataRescalingDescriptor.NO_RESCALE,
+                        0,
+                        parLevel,
+                        new String[] {tempDir.toString()},
+                        true,
+                        bufferSize);
+
+        withInputGates(
+                gates -> {
+                    assertThatThrownBy(() -> reader.readInputData(gates, 
spillingContext))
+                            .isInstanceOf(IOException.class);
+
+                    // The abort deleted the spill files and the per-recovery 
directory.
+                    assertThat(listSpillFiles(tempDir)).isEmpty();
+                    assertThat(listSpillDirs(tempDir)).isEmpty();
+                });
+    }
+
+    /**
+     * A stream state handle whose input stream cannot be opened, used to 
abort a channel-state read
+     * mid-recovery. A named (non-anonymous) class so it does not trip {@code
+     * StateHandleSerializationTest}, which forbids anonymous {@code 
StateObject} subclasses.
+     */
+    private static final class FailingStreamStateHandle extends 
EmptyStreamStateHandle {
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        public FSDataInputStream openInputStream() throws IOException {
+            throw new IOException("injected channel-state read failure");
+        }
+    }
+
+    private static List<Path> listSpillDirs(Path root) throws IOException {
+        try (Stream<Path> entries = Files.list(root)) {
+            return entries.filter(Files::isDirectory)
+                    .filter(p -> 
p.getFileName().toString().startsWith("flink-channel-spill-"))
+                    .collect(toList());
+        }
+    }
+
+    private static List<Path> listSpillFiles(Path root) throws IOException {
+        try (Stream<Path> entries = Files.walk(root)) {
+            return entries.filter(p -> 
p.getFileName().toString().endsWith(".bin"))
+                    .collect(toList());
+        }
+    }
+
     private Map<ResultSubpartitionInfo, List<Buffer>> collectBuffers(
             BufferWritingResultPartition[] resultPartitions) throws 
IOException {
         Map<ResultSubpartitionInfo, List<Buffer>> actual = new HashMap<>();

Reply via email to