Skip to content

Commit 9c784bf

Browse files
committed
Fix merge commutativity, fail fast on scan error, and add streaming restart note for per-partition bucket rescaling
1 parent 0d363ce commit 9c784bf

7 files changed

Lines changed: 72 additions & 29 deletions

File tree

‎docs/docs/maintenance/rescale-bucket.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ Please note that
6868
After these operations, partition `dt=2022-01-01` uses 4 buckets, `dt=2022-01-02` uses 8 buckets, and any
6969
new partitions will use the latest table-level default (8 buckets in this case).
7070
- During overwrite period, make sure there are no other jobs writing the same table/partition.
71+
- **Streaming jobs must be restarted after rescaling a partition.** The per-partition bucket mapping
72+
is loaded once when the streaming job starts (from the manifest files at that point in time). If a
73+
partition is rescaled while the streaming job is running, the job will continue routing rows using
74+
the old bucket count for that partition, which can cause rows to land in wrong buckets and lead to
75+
data correctness issues. The recommended workflow is: suspend the streaming job with a savepoint →
76+
perform the rescale overwrite → restart from the savepoint.
7177

7278
## Use Case
7379

‎paimon-core/src/main/java/org/apache/paimon/manifest/PartitionEntry.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,20 +83,26 @@ public int totalBuckets() {
8383
}
8484

8585
public PartitionEntry merge(PartitionEntry entry) {
86+
PartitionEntry newer = entry.lastFileCreationTime >= lastFileCreationTime ? entry : this;
87+
PartitionEntry older = newer == entry ? this : entry;
88+
8689
// Use the totalBuckets from the most recently created file. This correctly handles
8790
// the case where a partition has been overwritten with a different bucket count: the
8891
// newer files carry the new totalBuckets, and their creation time is always later.
89-
long newLastCreationTime = Math.max(lastFileCreationTime, entry.lastFileCreationTime);
92+
// When timestamps are equal (e.g., two files written in the same millisecond with
93+
// different bucket counts), we take the larger totalBuckets value. This makes merge
94+
// commutative and associative — a.merge(b) == b.merge(a)
9095
int newTotalBuckets =
91-
lastFileCreationTime >= entry.lastFileCreationTime
92-
? totalBuckets
93-
: entry.totalBuckets;
96+
newer.lastFileCreationTime == older.lastFileCreationTime
97+
? Math.max(newer.totalBuckets, older.totalBuckets)
98+
: newer.totalBuckets;
99+
94100
return new PartitionEntry(
95101
partition,
96102
recordCount + entry.recordCount,
97103
fileSizeInBytes + entry.fileSizeInBytes,
98104
fileCount + entry.fileCount,
99-
newLastCreationTime,
105+
newer.lastFileCreationTime,
100106
newTotalBuckets);
101107
}
102108

‎paimon-core/src/main/java/org/apache/paimon/table/SchemaBucketFileStoreTable.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,14 @@ public TableWriteImpl<?> newWrite(String commitUser, @Nullable Integer writeId)
6565
return wrapped().newWrite(commitUser, writeId, createRowKeyExtractor());
6666
}
6767

68+
@Override
69+
public TableWriteImpl<?> newWrite(
70+
String commitUser, @Nullable Integer writeId, RowKeyExtractor rowKeyExtractor) {
71+
// Always use the schema-bucket-based extractor; ignore the caller-supplied extractor
72+
// to ensure consistent per-partition bucket routing even when called via the 3-arg form.
73+
return wrapped().newWrite(commitUser, writeId, createRowKeyExtractor());
74+
}
75+
6876
@Override
6977
public FileStoreTable copy(Map<String, String> dynamicOptions) {
7078
return new SchemaBucketFileStoreTable(wrapped().copy(dynamicOptions));

‎paimon-core/src/main/java/org/apache/paimon/table/sink/PartitionBucketMapping.java‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,7 @@ public PartitionBucketMapping(
8383
* <p>For partitioned tables, the method reads {@link
8484
* org.apache.paimon.manifest.PartitionEntry}s, which aggregate manifest entries per partition
8585
* during the scan and therefore have a much smaller memory footprint than loading all data file
86-
* entries. If the scan fails for any reason, a fallback mapping with only the default bucket
87-
* count is returned.
86+
* entries. Any scan failure is propagated to the caller.
8887
*
8988
* @param table the {@link FileStoreTable} to load the mapping from
9089
* @return a {@link PartitionBucketMapping} reflecting the current bucket layout of the table
@@ -98,23 +97,21 @@ public static PartitionBucketMapping loadFromTable(FileStoreTable table) {
9897
}
9998

10099
public static PartitionBucketMapping loadFromScan(FileStoreScan scan, int defaultBuckets) {
101-
try {
102-
List<PartitionEntry> partitionEntries = scan.readPartitionEntries();
103-
Map<BinaryRow, Integer> partitionBucketMap = new HashMap<>();
104-
for (PartitionEntry entry : partitionEntries) {
105-
int totalBuckets = entry.totalBuckets();
106-
// Only store partitions whose bucket count differs from the default.
107-
// This keeps the map empty for partitions that have never been rescaled,
108-
// avoiding per-partition BinaryRow copies and Integer allocations entirely.
109-
if (totalBuckets > 0 && totalBuckets != defaultBuckets) {
110-
partitionBucketMap.put(entry.partition().copy(), totalBuckets);
111-
}
112-
}
113-
114-
return new PartitionBucketMapping(defaultBuckets, partitionBucketMap);
115-
} catch (Exception e) {
100+
if (scan == null) {
116101
return new PartitionBucketMapping(defaultBuckets, Collections.emptyMap());
117102
}
103+
List<PartitionEntry> partitionEntries = scan.readPartitionEntries();
104+
Map<BinaryRow, Integer> partitionBucketMap = new HashMap<>();
105+
for (PartitionEntry entry : partitionEntries) {
106+
int totalBuckets = entry.totalBuckets();
107+
// Only store partitions whose bucket count differs from the default.
108+
// This keeps the map empty for partitions that have never been rescaled,
109+
// avoiding per-partition BinaryRow copies and Integer allocations entirely.
110+
if (totalBuckets > 0 && totalBuckets != defaultBuckets) {
111+
partitionBucketMap.put(entry.partition().copy(), totalBuckets);
112+
}
113+
}
114+
return new PartitionBucketMapping(defaultBuckets, partitionBucketMap);
118115
}
119116

120117
/**

‎paimon-core/src/test/java/org/apache/paimon/manifest/PartitionEntryTest.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,21 @@ public void testMergeWithDeleteEntryPreservesNewerTotalBuckets() {
103103
}
104104

105105
@Test
106-
public void testMergeWithEqualCreationTimeTakesFirstTotalBuckets() {
107-
// When creation times are equal, the receiver's (this) totalBuckets is used.
108-
// This is the tie-breaking behavior: the entry already accumulated takes precedence.
106+
public void testMergeWithEqualCreationTimeIsCommutative() {
107+
// When creation times are equal, merge must be commutative: a.merge(b) == b.merge(a).
108+
// The tie-break takes the larger totalBuckets so that the parallel, non-deterministic
109+
// aggregation in readPartitionEntries() always produces the same result regardless of
110+
// manifest processing order.
109111
PartitionEntry a = entry(1, 2, 1000L);
110112
PartitionEntry b = entry(1, 4, 1000L);
111113

112-
PartitionEntry result = a.merge(b);
113-
assertThat(result.totalBuckets())
114-
.isEqualTo(2); // 'a' wins tie (lastCreationTime >= entry's)
115-
assertThat(result.fileCount()).isEqualTo(2);
114+
PartitionEntry ab = a.merge(b);
115+
PartitionEntry ba = b.merge(a);
116+
117+
assertThat(ab.totalBuckets()).isEqualTo(4); // max(2, 4) = 4
118+
assertThat(ba.totalBuckets()).isEqualTo(4); // max(4, 2) = 4, commutative
119+
assertThat(ab.fileCount()).isEqualTo(2);
120+
assertThat(ba.fileCount()).isEqualTo(2);
116121
}
117122

118123
@Test

‎paimon-core/src/test/java/org/apache/paimon/table/sink/PartitionBucketMappingTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,16 @@
1919
package org.apache.paimon.table.sink;
2020

2121
import org.apache.paimon.data.BinaryRow;
22+
import org.apache.paimon.operation.FileStoreScan;
2223

2324
import org.junit.jupiter.api.Test;
25+
import org.mockito.Mockito;
2426

2527
import java.util.HashMap;
2628
import java.util.Map;
2729

2830
import static org.assertj.core.api.Assertions.assertThat;
31+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2932

3033
/** Tests for {@link PartitionBucketMapping}. */
3134
public class PartitionBucketMappingTest {
@@ -60,6 +63,19 @@ public void testExplicitPartitionMapping() {
6063
assertThat(mapping.resolveNumBuckets(partC)).isEqualTo(16);
6164
}
6265

66+
@Test
67+
public void testLoadFromScanPropagatesException() {
68+
// Simulate a scan that throws (e.g. corrupted manifest, transient I/O error).
69+
// loadFromScan must fail fast so the job does not silently write to wrong buckets.
70+
FileStoreScan failingScan = Mockito.mock(FileStoreScan.class);
71+
Mockito.when(failingScan.readPartitionEntries())
72+
.thenThrow(new RuntimeException("simulated manifest scan failure"));
73+
74+
assertThatThrownBy(() -> PartitionBucketMapping.loadFromScan(failingScan, 8))
75+
.isInstanceOf(RuntimeException.class)
76+
.hasMessageContaining("simulated manifest scan failure");
77+
}
78+
6379
private static BinaryRow partition(int value) {
6480
return BinaryRow.singleColumn(value);
6581
}

‎paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/coordinator/TableWriteCoordinator.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,11 @@ public void checkpoint() {
199199

200200
private void loadPartitionBucketMapping() {
201201
int defaultNumBuckets = table.schema().numBuckets();
202+
// Note: `scan` is shared between this method (called during refresh/checkpoint) and the
203+
// `scan(ScanCoordinationRequest)` method (which calls scan.withPartitionBucket(...)).
204+
// Both callers always invoke scan.withSnapshot(...) before using the scan, so the shared
205+
// state is safe. The partition-bucket filter set by withPartitionBucket is applied per
206+
// plan() call and does not bleed across invocations.
202207
this.partitionBucketMapping = PartitionBucketMapping.loadFromScan(scan, defaultNumBuckets);
203208
}
204209

0 commit comments

Comments
 (0)