Skip to content

Commit 892b20c

Browse files
yujun777yujun777
committed
[fix](ivm) Rebuild the whole MV when a change mixes windowed and unwindowed partitions
The conservative branch added for partition_sync_limit judged the change by whether the selection came out empty, so a statement that touches a partition inside the window and one outside it at once slipped through: the inside half filled the selection while the outside half contributed nothing, because the window leaves it out of the mapping. Only the newer MV partition got the barrier, and the rows of the outside half -- still sitting in the MV partition that read them -- were never rebuilt, with no incremental delta to repair them if the window is widened later. Key changes: - Require the mapping to describe every changed base partition, not merely to select something: a name the windowed mapping leaves out cannot be told apart from a partition no MV partition reads, so the whole MV is rebuilt - Keep that judgement behind the same partition_sync_limit guard: without a limit the mapping is complete, and a partition it leaves out really is one no MV partition reads - Log the names the mapping did not describe, which is what makes such a change readable in the FE log Unit Test: - IvmBaselineRebuildTest.testChangeThatMixesInWindowAndOutOfWindowPartitionsRebuildsTheWholeMv truncates one partition inside the window and one outside it in a single statement (the assertion fails with the empty-selection check, which is what the earlier round added)
1 parent 0f119ee commit 892b20c

2 files changed

Lines changed: 62 additions & 8 deletions

File tree

‎fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -818,12 +818,16 @@ private Optional<Set<String>> selectAffectedMvPartitions(BaseTableInfo baseTable
818818
calculatePartitionMappings(Maps.newHashMap());
819819
Set<String> res = Sets.newHashSet();
820820
boolean pctTableMapped = false;
821+
// Every base partition this table's part of the mapping describes, which is what the selection
822+
// below is only allowed to trust when it covers the whole change.
823+
Set<String> mappedBasePartitions = Sets.newHashSet();
821824
for (Entry<String, Map<MTMVRelatedTableIf, Set<String>>> mapping : partitionMappings.entrySet()) {
822825
for (Entry<MTMVRelatedTableIf, Set<String>> tableMapping : mapping.getValue().entrySet()) {
823826
if (!tableMapping.getKey().equals(pctTable)) {
824827
continue;
825828
}
826829
pctTableMapped = true;
830+
mappedBasePartitions.addAll(tableMapping.getValue());
827831
if (!Collections.disjoint(tableMapping.getValue(), changedBasePartitions.keySet())) {
828832
res.add(mapping.getKey());
829833
}
@@ -843,16 +847,22 @@ private Optional<Set<String>> selectAffectedMvPartitions(BaseTableInfo baseTable
843847
+ "baseTable={}, mv={}", baseTableInfo, name);
844848
return Optional.empty();
845849
}
846-
// An empty selection is only trustworthy while the mapping covers every base partition. With a
847-
// partition_sync_limit in effect it does not: the window leaves out the partitions it dropped,
848-
// and one of those can still have its rows in an MV partition -- shrinking the window does not
849-
// touch the MV's own partitions, and widening it again makes partition sync keep them. Those
850-
// two readings cannot be told apart here, so the whole MV is rebuilt instead; without a limit
851-
// the mapping is complete, and an empty selection really does mean no MV partition holds them.
852-
if (res.isEmpty() && MTMVPartitionUtil.isPartitionSyncLimitActive(mvProperties)) {
850+
// A selection is only trustworthy while the mapping describes every base partition that
851+
// changed. With a partition_sync_limit in effect it does not: the window leaves out the
852+
// partitions it dropped, and one of those can still have its rows in an MV partition --
853+
// shrinking the window does not touch the MV's own partitions, and widening it again makes
854+
// partition sync keep them. A name the mapping leaves out cannot be told apart from a
855+
// partition no MV partition reads, so the whole MV is rebuilt instead. Requiring the whole
856+
// change to be described, rather than only a non-empty selection, is what covers a change
857+
// that mixes a partition inside the window with one outside it: the inside half would
858+
// otherwise fill the selection and hide the missing half. Without a limit the mapping is
859+
// complete, and a partition it leaves out really is one no MV partition reads.
860+
if (MTMVPartitionUtil.isPartitionSyncLimitActive(mvProperties)
861+
&& !mappedBasePartitions.containsAll(changedBasePartitions.keySet())) {
853862
LOG.info("Changed base partitions are outside the partition_sync_limit window and the MV may "
854863
+ "still hold their rows, rebuild the whole MV. baseTable={}, changedPartitions={}, "
855-
+ "mv={}", baseTableInfo, changedBasePartitions.keySet(), name);
864+
+ "undescribed={}, mv={}", baseTableInfo, changedBasePartitions.keySet(),
865+
Sets.difference(changedBasePartitions.keySet(), mappedBasePartitions), name);
856866
return Optional.empty();
857867
}
858868
return Optional.of(res);

‎fe/fe-core/src/test/java/org/apache/doris/mtmv/ivm/IvmBaselineRebuildTest.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,50 @@ public void testChangedPartitionOutsideTheSyncWindowRebuildsTheWholeMv() throws
212212
Assertions.assertTrue(mtmv.getIvmInfo().requiresCompleteBaselineRebuild());
213213
}
214214

215+
/**
216+
* The same window, with a change that touches a partition inside it and one outside it at once: the
217+
* partition inside fills the selection, and the one outside contributes nothing because the window
218+
* left it out of the mapping. Judging the change by "was anything selected" would mark only the MV
219+
* partition backed by the inside half, and the rows of the outside half -- which the MV partition for
220+
* it still holds -- would never be rebuilt.
221+
*/
222+
@Test
223+
public void testChangeThatMixesInWindowAndOutOfWindowPartitionsRebuildsTheWholeMv() throws Exception {
224+
String db = "ivm_baseline_sync_window_mixed";
225+
String thisYear = LocalDate.now().withDayOfYear(1).toString();
226+
String nextYear = LocalDate.now().withDayOfYear(1).plusYears(1).toString();
227+
createDatabaseAndUse(db);
228+
createTable("CREATE TABLE " + db + ".ivm_base (\n"
229+
+ " dt date NOT NULL,\n"
230+
+ " k1 int,\n"
231+
+ " v1 int\n"
232+
+ ")\n"
233+
+ "DUPLICATE KEY(dt, k1)\n"
234+
+ "PARTITION BY RANGE(dt) (\n"
235+
+ " PARTITION p202001 VALUES [('2020-01-01'), ('2020-02-01')),\n"
236+
+ " PARTITION p202002 VALUES [('2020-02-01'), ('2020-03-01')),\n"
237+
+ " PARTITION pThisYear VALUES [('" + thisYear + "'), ('" + nextYear + "'))\n"
238+
+ ")\n"
239+
+ "DISTRIBUTED BY HASH(k1) BUCKETS 1\n"
240+
+ "PROPERTIES ('replication_num' = '1', 'binlog.enable' = 'true', 'binlog.format' = 'ROW')");
241+
createMvByNereids("CREATE MATERIALIZED VIEW ivm_mv\n"
242+
+ "BUILD DEFERRED REFRESH INCREMENTAL ON MANUAL\n"
243+
+ "PARTITION BY(dt)\n"
244+
+ "DISTRIBUTED BY RANDOM BUCKETS 1\n"
245+
+ "PROPERTIES ('replication_num' = '1')\n"
246+
+ "AS SELECT dt, k1, v1 FROM ivm_base");
247+
MTMV mtmv = getMtmv(db);
248+
Assertions.assertEquals(3, mtmv.getPartitionNames().size());
249+
250+
executeSql("ALTER MATERIALIZED VIEW ivm_mv SET ('partition_sync_limit' = '1',"
251+
+ " 'partition_sync_time_unit' = 'YEAR')");
252+
// One statement, so the marker sees both partitions together: pThisYear is inside the window while
253+
// p202001 is not, which is exactly the mix a non-empty selection must not be allowed to hide.
254+
executeSql("TRUNCATE TABLE ivm_base PARTITION(p202001, pThisYear)");
255+
256+
Assertions.assertTrue(mtmv.getIvmInfo().requiresCompleteBaselineRebuild());
257+
}
258+
215259
/**
216260
* The partition mapping is built from the MV's PCT tables only. A changed partition of a joined table
217261
* the MV's partition column does not reach is invisible to it, and missing such a change leaves rows

0 commit comments

Comments
 (0)