-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[fix](stream) Mark streams stale when base tables are dropped #66287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
28af68e
6282603
cc88b06
7ea0064
a7850ad
9c8100c
25d0c27
6f451cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,8 @@ | |
| package org.apache.doris.catalog.stream; | ||
|
|
||
| import org.apache.doris.catalog.Column; | ||
| import org.apache.doris.catalog.Database; | ||
| import org.apache.doris.catalog.Env; | ||
| import org.apache.doris.catalog.Table; | ||
| import org.apache.doris.catalog.TableIf; | ||
| import org.apache.doris.common.UserException; | ||
|
|
@@ -37,6 +39,8 @@ | |
| import java.util.Map; | ||
|
|
||
| public abstract class BaseTableStream extends Table { | ||
| private static final String BASE_TABLE_NOT_FOUND_STALE_REASON = "Base table does not exist"; | ||
|
|
||
| public enum StreamScanType { | ||
| APPEND_ONLY, | ||
| MIN_DELTA, | ||
|
|
@@ -113,10 +117,33 @@ public BaseTableStream(String streamName, List<Column> fullSchema, TableIf baseT | |
| } | ||
|
|
||
| public TableIf getBaseTableNullable() { | ||
| if (baseTable == null) { | ||
| baseTable = baseTableInfo.getTableNullable(); | ||
| TableIf cachedBaseTable = baseTable; | ||
| if (isBaseTableAvailable(cachedBaseTable)) { | ||
| return cachedBaseTable; | ||
| } | ||
| if (cachedBaseTable != null) { | ||
| baseTable = null; | ||
| } | ||
| TableIf resolvedBaseTable = baseTableInfo.getTableNullable(); | ||
| if (!isBaseTableAvailable(resolvedBaseTable)) { | ||
| return null; | ||
| } | ||
| return baseTable; | ||
| baseTable = resolvedBaseTable; | ||
| return resolvedBaseTable; | ||
| } | ||
|
|
||
| private boolean isBaseTableAvailable(TableIf candidate) { | ||
| if (candidate == null || candidate instanceof Table && ((Table) candidate).isDropped) { | ||
| return false; | ||
| } | ||
| if (!baseTableInfo.isInternalTable()) { | ||
| return true; | ||
| } | ||
| // Table recovery publishes into database maps before clearing the table flag, while database recovery clears | ||
| // member-table flags before publishing the database. Require both catalog mappings to reject either window. | ||
| Database database = Env.getCurrentInternalCatalog().getDbNullable(baseTableInfo.getDbId()); | ||
| return database != null && !database.isDropped() | ||
| && database.getTable(baseTableInfo.getTableId()).orElse(null) == candidate; | ||
| } | ||
|
|
||
| public void setProperties(Map<String, String> properties) throws org.apache.doris.common.AnalysisException { | ||
|
|
@@ -139,23 +166,35 @@ public StreamScanType getStreamScanType() { | |
| } | ||
|
|
||
| public boolean isDisabled() { | ||
| return disabled; | ||
| return isDisabled(getBaseTableNullable()); | ||
| } | ||
|
|
||
| boolean isDisabled(TableIf availableBaseTable) { | ||
| return disabled || availableBaseTable == null; | ||
| } | ||
|
|
||
| public void setDisabled(boolean disabled) { | ||
| this.disabled = disabled; | ||
| } | ||
|
|
||
| public boolean isStale() { | ||
| return stale; | ||
| return isStale(getBaseTableNullable()); | ||
| } | ||
|
|
||
| boolean isStale(TableIf availableBaseTable) { | ||
| return stale || availableBaseTable == null; | ||
| } | ||
|
|
||
| public void setStale(boolean stale) { | ||
| this.stale = stale; | ||
| } | ||
|
|
||
| public String getStaleReason() { | ||
| return staleReason; | ||
| return getStaleReason(getBaseTableNullable()); | ||
| } | ||
|
|
||
| String getStaleReason(TableIf availableBaseTable) { | ||
| return availableBaseTable == null ? BASE_TABLE_NOT_FOUND_STALE_REASON : staleReason; | ||
| } | ||
|
|
||
| public void setStaleReason(String staleReason) { | ||
|
|
@@ -198,7 +237,23 @@ public TableIf getBaseTableOrNereidsAnalysisException() throws AnalysisException | |
| } | ||
|
|
||
| public List<String> getBaseTableFullQualifiers() { | ||
| return baseTableInfo.getFullQualifiers(); | ||
| return getBaseTableFullQualifiers(getBaseTableNullable()); | ||
| } | ||
|
|
||
| List<String> getBaseTableFullQualifiers(TableIf availableBaseTable) { | ||
| TableIf displayBaseTable = availableBaseTable; | ||
| if (displayBaseTable == null && baseTableInfo.isInternalTable()) { | ||
| displayBaseTable = Env.getCurrentRecycleBin().getRecycledTableNullable( | ||
| baseTableInfo.getDbId(), baseTableInfo.getTableId()); | ||
| } | ||
| if (baseTableInfo.isInternalTable()) { | ||
| return ImmutableList.of( | ||
| baseTableInfo.getCtlName(), | ||
| Env.getCurrentInternalCatalog().getDb(baseTableInfo.getDbId()) | ||
| .map(db -> db.getFullName()).orElse(baseTableInfo.getDbName()), | ||
| displayBaseTable == null ? baseTableInfo.getTableName() : displayBaseTable.getName()); | ||
| } | ||
| return displayBaseTable == null ? baseTableInfo.getFullQualifiers() : displayBaseTable.getFullQualifiers(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Resolve recycled qualifiers without the detached table's database pointer This is distinct from the existing same-ID table-rename thread: after Derive the internal catalog/database qualifiers from stable IDs/current live-or-recycled database identity, and preserve only the latest table name independently of the recycled object's lifetime; cover drop-base -> rename-database, reload, and recycle erasure.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed the detached-table failure in |
||
| } | ||
|
|
||
| public TableStreamBaseTableInfo getBaseTableInfo() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -350,32 +350,22 @@ public void fillTableStreamValuesMetadataResult(List<TRow> dataBatch) { | |
| // STREAM_COMMENT | ||
| trow.addToColumnValue(new TCell().setStringVal(stream.getComment())); | ||
| TableIf baseTable = stream.getBaseTableNullable(); | ||
| if (baseTable == null) { | ||
| // BASE_TABLE_NAME | ||
| trow.addToColumnValue(new TCell().setStringVal("N/A")); | ||
| // BASE_TABLE_DB | ||
| trow.addToColumnValue(new TCell().setStringVal("N/A")); | ||
| // BASE_TABLE_CTL | ||
| trow.addToColumnValue(new TCell().setStringVal("N/A")); | ||
| // BASE_TABLE_TYPE | ||
| trow.addToColumnValue(new TCell().setStringVal("N/A")); | ||
| } else { | ||
| List<String> baseTableQualifiers = baseTable.getFullQualifiers(); | ||
| // BASE_TABLE_NAME | ||
| trow.addToColumnValue(new TCell().setStringVal(baseTableQualifiers.get(2))); | ||
| // BASE_TABLE_DB | ||
| trow.addToColumnValue(new TCell().setStringVal(baseTableQualifiers.get(1))); | ||
| // BASE_TABLE_CTL | ||
| trow.addToColumnValue(new TCell().setStringVal(baseTableQualifiers.get(0))); | ||
| // BASE_TABLE_TYPE | ||
| trow.addToColumnValue(new TCell().setStringVal(baseTable.getType().name())); | ||
| } | ||
| List<String> baseTableQualifiers = stream.getBaseTableFullQualifiers(baseTable); | ||
| // BASE_TABLE_NAME | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Use live qualifiers while the base table is available
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||
| trow.addToColumnValue(new TCell().setStringVal(baseTableQualifiers.get(2))); | ||
| // BASE_TABLE_DB | ||
| trow.addToColumnValue(new TCell().setStringVal(baseTableQualifiers.get(1))); | ||
| // BASE_TABLE_CTL | ||
| trow.addToColumnValue(new TCell().setStringVal(baseTableQualifiers.get(0))); | ||
| // BASE_TABLE_TYPE | ||
| trow.addToColumnValue(new TCell().setStringVal( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Derive the row from one availability snapshot
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||
| baseTable == null ? "N/A" : baseTable.getType().name())); | ||
| // ENABLED | ||
| trow.addToColumnValue(new TCell().setBoolVal(!stream.isDisabled())); | ||
| trow.addToColumnValue(new TCell().setBoolVal(!stream.isDisabled(baseTable))); | ||
| // IS_STALE | ||
| trow.addToColumnValue(new TCell().setBoolVal(stream.isStale())); | ||
| trow.addToColumnValue(new TCell().setBoolVal(stream.isStale(baseTable))); | ||
| // STALE_REASON | ||
| trow.addToColumnValue(new TCell().setStringVal(stream.getStaleReason())); | ||
| trow.addToColumnValue(new TCell().setStringVal(stream.getStaleReason(baseTable))); | ||
| dataBatch.add(trow); | ||
| } finally { | ||
| stream.readUnlock(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -224,7 +224,10 @@ public static Pair<Set<TableIf>, Set<TableIf>> getBaseTableFromQuery(String quer | |
| try { | ||
| NereidsPlanner planner = new NereidsPlanner(ctx.getStatementContext()); | ||
| planner.planWithLock(logicalPlan, PhysicalProperties.ANY, ExplainLevel.ANALYZED_PLAN); | ||
| return Pair.of(Sets.newHashSet(ctx.getStatementContext().getTables().values()), | ||
| Set<TableIf> baseTables = Sets.newHashSet(ctx.getStatementContext().getTables().values()); | ||
| // Implicit dependencies are all-level tables, not relations written at the first query level. | ||
| baseTables.addAll(ctx.getStatementContext().getImplicitTableDependencies()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Migrate persisted stream dependencies on upgrade These additions repair newly analyzed creation and refresh, but an MTMV image can already contain stream |
||
| return Pair.of(baseTables, | ||
| Sets.newHashSet(ctx.getStatementContext().getOneLevelTables().values())); | ||
| } finally { | ||
| ctx.setStatementContext(original); | ||
|
|
@@ -448,6 +451,8 @@ public static MTMVAnalyzeQueryInfo analyzeQuery(ConnectContext ctx, Map<String, | |
| } | ||
|
|
||
| Set<TableIf> baseTables = Sets.newHashSet(statementContext.getTables().values()); | ||
| // Implicit dependencies are all-level tables, not relations written at the first query level. | ||
| baseTables.addAll(statementContext.getImplicitTableDependencies()); | ||
| Set<TableIf> oneLevelTables = Sets.newHashSet(statementContext.getOneLevelTables().values()); | ||
| for (TableIf table : baseTables) { | ||
| if (table.isTemporary()) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,9 @@ | |
|
|
||
| package org.apache.doris.mtmv; | ||
|
|
||
| import org.apache.doris.catalog.TableIf; | ||
| import org.apache.doris.catalog.stream.BaseTableStream; | ||
| import org.apache.doris.common.AnalysisException; | ||
| import org.apache.doris.datasource.CatalogMgr; | ||
| import org.apache.doris.persist.gson.GsonPostProcessable; | ||
|
|
||
|
|
@@ -104,6 +107,12 @@ public void compatible(CatalogMgr catalogMgr) throws Exception { | |
| compatible(catalogMgr, baseTables); | ||
| compatible(catalogMgr, baseViews); | ||
| compatible(catalogMgr, baseTablesOneLevel); | ||
| addStreamBaseTables(baseTables); | ||
| if (CollectionUtils.isEmpty(baseTablesOneLevelAndFromView)) { | ||
| // Preserve the existing fallback for older images in a separate set before adding implicit stream bases. | ||
| baseTablesOneLevelAndFromView = new HashSet<>(getBaseTablesOneLevel()); | ||
| } | ||
| addStreamBaseTables(baseTablesOneLevelAndFromView); | ||
| } | ||
|
|
||
| private void compatible(CatalogMgr catalogMgr, Set<BaseTableInfo> infos) throws Exception { | ||
|
|
@@ -114,4 +123,28 @@ private void compatible(CatalogMgr catalogMgr, Set<BaseTableInfo> infos) throws | |
| baseTableInfo.compatible(catalogMgr); | ||
| } | ||
| } | ||
|
|
||
| private void addStreamBaseTables(Set<BaseTableInfo> infos) throws Exception { | ||
| if (CollectionUtils.isEmpty(infos)) { | ||
| return; | ||
| } | ||
| // Older images may contain only the stream relation; add its stable base so freshness and invalidation survive | ||
| // an upgrade without inventing a historical snapshot for the newly discovered dependency. | ||
| for (BaseTableInfo info : new HashSet<>(infos)) { | ||
| TableIf table; | ||
| try { | ||
| table = MTMVUtil.getTable(info); | ||
| } catch (AnalysisException e) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Do not complete migration while the stream is missing For an old image where |
||
| continue; | ||
| } | ||
| if (table instanceof BaseTableStream) { | ||
| TableIf baseTable = ((BaseTableStream) table).getBaseTableNullable(); | ||
| if (baseTable == null) { | ||
| throw new AnalysisException( | ||
| "Failed to resolve stream base table during MTMV compatibility: " + info); | ||
| } | ||
| infos.add(new BaseTableInfo(baseTable)); | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Collect the stable-ID base object, not its display name
Create
son A asorig, rename the same-ID A toold, recoverably drop A, then create B namedold. This new recycle-backed display helper returnsold, soCollectRelationcaches B andStatementContext.lock()locks B; before this diff it returned persistedorig, so collection failed instead. A concurrentRECOVER TABLE old AS newcan publish/unmark A while recovery still owns A's write lock (it takes database/recycle/A locks, not B or the stream), andBindRelation.makeOlapTableStreamScan()then resolves A by the persisted ID. The wrapper now reads A even though A is absent fromplannerResources; a concurrent drop can even makegetStreamUpdate()re-resolve null and dereference.getPartition(...).Please have collection resolve/cache the exact stable-ID base object (failing while that ID is unavailable), then reuse or fence that same snapshot during binding. Cover
orig -> old -> drop -> B(old)with a latch-controlled collect/lock/recover-as/bind test and an end-to-end replacement query.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in
a0238b249e6.collectFromTableStream()now resolves the base table throughgetBaseTableNullable(), which uses the persisted stable(dbId, tableId), and fails collection when that exact base object is unavailable instead of resolving a same-name replacement through the display qualifiers. The resolved base object is then included in planner locking before binding. The separate qualifier-key collision reported inr3746321533is distinct and still needs a follow-up fix.