github-actions[bot] commented on code in PR #68193:
URL: https://github.com/apache/doris/pull/68193#discussion_r4045710013
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -598,6 +621,33 @@ public void alterIvmInfo(IvmInfo ivmInfo) {
}
}
+ /**
+ * Read under the MV lock, like {@link #getIvmInfo()}: the map may be null
before
+ * {@link #gsonPostProcess()} has run, and a reader must never see a
half-applied replay payload.
+ */
+ public Map<String, MTMVPartitionState> getPartitionStates() {
+ writeMvLock();
+ try {
+ if (partitionStates == null) {
+ partitionStates = Maps.newLinkedHashMap();
+ }
+ return partitionStates;
Review Comment:
[P2] Keep partition-state ownership inside `mvRwLock`
This returns the live map after the `finally` releases `mvRwLock`, and each
value is mutable too. A caller using the pattern in the new tests
(`getPartitionStates().put(...)`) can therefore add/remove entries or change an
epoch while `addTaskResult()` is copying the same `LinkedHashMap` for the
journal, producing a `ConcurrentModificationException` or a mixed snapshot. If
replay replaces the field first, the retained reference instead accepts a
silently lost update. Since this API is the persistence foundation for the
follow-up invalidation/alignment code, please return a deep
detached/unmodifiable snapshot for reads and add lock-owning MTMV mutation
methods that mutate and enqueue `ALTER_PARTITION_STATES` under the same write
lock.
##########
fe/fe-core/src/test/java/org/apache/doris/mtmv/AlterMTMVTest.java:
##########
@@ -403,6 +405,44 @@ public void testAlterIvmInfoPersistence() throws Exception
{
Assertions.assertEquals(schemaChangeVersion,
mtmv.getSchemaChangeVersion());
}
+ @Test
+ public void testReplayAlterPartitionStates() throws Exception {
+ Config.enable_table_stream = true;
+ createDatabaseAndUse("alter_partition_states_test");
+ createTable("CREATE TABLE alter_partition_states_test.states_base (k1
int, v1 int)\n"
+ + "DUPLICATE KEY(k1)\n"
+ + "DISTRIBUTED BY HASH(k1) BUCKETS 1\n"
+ + "PROPERTIES ('replication_num' = '1', 'binlog.enable' =
'true', 'binlog.format' = 'ROW')");
+ createMvByNereids("CREATE MATERIALIZED VIEW states_mv\n"
+ + " BUILD DEFERRED REFRESH INCREMENTAL ON MANUAL\n"
+ + " DISTRIBUTED BY RANDOM BUCKETS 2\n"
+ + " PROPERTIES ('replication_num' = '1')\n"
+ + " AS SELECT k1, v1 FROM states_base");
+
+ MTMV mtmv = (MTMV) Env.getCurrentInternalCatalog()
+ .getDb("alter_partition_states_test").get()
+ .getTableOrMetaException("states_mv");
+ String partitionName = mtmv.getPartitionNames().iterator().next();
+
+ MTMVPartitionState state = new MTMVPartitionState(0, 1);
+ Map<String, MTMVPartitionState> states = new LinkedHashMap<>();
+ states.put(partitionName, state);
+ TableNameInfo tableName = new TableNameInfo(mtmv.getQualifiedDbName(),
mtmv.getName());
+ AlterMTMV replayAlter = new AlterMTMV(tableName,
MTMVAlterOpType.ALTER_PARTITION_STATES);
+ replayAlter.setPartitionStates(states);
+ // The live map keeps moving after the payload was taken; the payload
must not follow it.
+ state.setLatestEpoch(7);
+ // The MV starts without any state, so only the replayed payload can
put it there.
+ mtmv.alterPartitionStates(Map.of());
+
+ Env.getCurrentEnv().getAlterInstance().processAlterMTMV(replayAlter,
true);
Review Comment:
[P2] Exercise the journal wire round trip before replay
This calls `processAlterMTMV` with the same in-memory object created above,
and the ADD_TASK tests likewise inspect an object captured by a mocked
`submitEdit`. As a result, removing or mis-serializing the new op/`pst`
member—or collapsing the required absent-versus-empty distinction—would leave
every new replay test green even though restart/failover is this PR's main
deliverable. Please round-trip `AlterMTMV` through `write`/`read` (or
`JournalEntity`) before replay and cover present nonempty, present empty
(clear), and absent old payload (preserve).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
