You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "alpass163gmail (via GitHub)" <gi...@apache.org> on 2026/07/06 10:38:05 UTC
[PR] Cyb/metadata table [iotdb]
alpass163gmail opened a new pull request, #18127:
URL: https://github.com/apache/iotdb/pull/18127
This PR introduces a lease-based self-fencing framework to enable high
availability for table metadata operations. It prevents DataNodes from
serving stale schema during network partitions.
Problem
Currently, in a cluster deployment (e.g., 1 ConfigNode, 3 DataNodes),
metadata operations lack true high availability. If a single DataNode (DN)
crashes or experiences a network partition, execution of DDL procedures
(such as Create/Alter/Drop Table, View management, TTL adjustments, and
Drop Database) will directly fail and trigger a rollback.
Solution
Lease Mechanism
- MetadataLeaseManager (DataNode): Tracks lease via ConfigNode heartbeats.
Uses monotonic clock. Self-fences (clears cache, blocks reads/writes) if no
heartbeat received within metadata_lease_fence_ms (T_fence).
- MetadataLeaseFencedException: Thrown when operations are blocked on a
fenced DataNode.
Broadcast Coordination
- DataNodeContactTracker (ConfigNode): Records time of last successful
heartbeat response per DataNode. Separately maintained from load-balancing
samples to ensure correctness.
- MetadataBroadcastVerdict: Pure decision logic — PROCEED if all unacked
DataNodes have been silent for T_proceed = T_fence + margin, WAIT
otherwise, FAIL when retry budget exhausted.
- ClusterCachePropagator: Broadcasts cache invalidations with retry loop,
waiting up to T_proceed for unresponsive DataNodes to prove self-fenced.
Schema Change Integration
- Procedures now propagate metadata invalidations via
ClusterCachePropagator before proceeding.
- Pre-deletion marker (PreDeleteTsTable) added for safe table state
transitions.
- Rollback mechanism (RollbackPreDeleteTablePlan) for failed schema
changes.
Configuration
- New config: metadata_lease_fence_ms (default in
iotdb-system.properties.template).
Testing
- Unit tests for MetadataLeaseManager, DataNodeContactTracker,
MetadataBroadcastVerdict, ClusterCachePropagator
- Lease integration tests for DataNodeTableCache, PartitionCache,
ClusterAuthorityFetcher
- New HA IT: IoTDBTableDDLHAIT
Key Components Added
- MetadataLeaseManager (DataNode): Tracks the lease and performs
self-fencing upon expiration.
- DataNodeContactTracker (ConfigNode): Tracks the last successful heartbeat
timestamp for each DataNode.
- ClusterCachePropagator (ConfigNode): Broadcasts cache invalidations with
a fencing-aware retry mechanism.
- MetadataBroadcastVerdict (ConfigNode): Decides when unacknowledged
DataNodes are logically safe to skip.
- MetadataLeaseFencedException (node-commons): The explicit exception
thrown on a fenced DataNode.
- PreDeleteTsTable (node-commons): Represents the marker table state to
ensure safe schema deletion.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cyb/metadata table [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 merged PR #18127:
URL: https://github.com/apache/iotdb/pull/18127
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cyb/metadata table [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 commented on code in PR #18127:
URL: https://github.com/apache/iotdb/pull/18127#discussion_r3534093133
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/RemoveDataNodeHandler.java:
##########
@@ -455,6 +456,9 @@ public void removeDataNodePersistence(List<TDataNodeLocation> removedDataNodes)
PartitionMetrics.unbindDataNodePartitionMetricsWhenUpdate(
MetricService.getInstance(),
NodeUrlUtils.convertTEndPointUrl(dataNodeLocation.getClientRpcEndPoint()));
+ // Drop the removed DataNode's metadata-lease contact/capability state so it is not retained,
+ // and a future DataNode reusing the id cannot inherit stale fencing history.
Review Comment:
id will always increase, no new DN will reuse removed DN's id
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java:
##########
@@ -2416,12 +2416,17 @@ public TDeleteTableDeviceResp deleteDevices(
}
}
+ // only care about the AbstractAlterOrDropTableProcedure(except the drop table/view)
+ // and the DeleteDatabaseProcedure
Review Comment:
explain why we need to exclude the drop table/view procedure
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/table/AbstractAlterOrDropTableProcedure.java:
##########
@@ -91,17 +93,22 @@ protected void preRelease(final ConfigNodeProcedureEnv env) {
}
protected void preRelease(final ConfigNodeProcedureEnv env, final @Nullable String oldName) {
- final Map<Integer, TSStatus> failedResults =
- SchemaUtils.preReleaseTable(database, table, env.getConfigManager(), oldName);
-
- if (!failedResults.isEmpty()) {
- // All dataNodes must clear the related schema cache
+ // Proceed once every unreachable DataNode is provably self-fenced instead of hard-failing the
+ // DDL: a fenced DataNode fails closed on its now-stale table cache and resyncs on lease
+ // recovery, so it cannot serve dirty schema. Only fail if an unacked DataNode is not provably
+ // fenced (it may still be serving clients).
+ final TUpdateTableReq req = SchemaUtils.BuildPreUpdateTableReq(database, table, oldName);
Review Comment:
```suggestion
final TUpdateTableReq req = SchemaUtils.buildPreUpdateTableReq(database, table, oldName);
```
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/table/AbstractAlterOrDropTableProcedure.java:
##########
@@ -140,18 +147,21 @@ protected void rollbackPreRelease(final ConfigNodeProcedureEnv env) {
protected void rollbackPreRelease(
final ConfigNodeProcedureEnv env, final @Nullable String tableName) {
- final Map<Integer, TSStatus> failedResults =
- SchemaUtils.rollbackPreRelease(
- database, table.getTableName(), env.getConfigManager(), tableName);
-
- if (!failedResults.isEmpty()) {
- // All dataNodes must clear the related schema cache
+ // A down DataNode must not block rollback either: proceed past provably-fenced DataNodes (which
+ // resync on recovery) and only fail on an unacked DataNode that is not provably fenced.
+ final TUpdateTableReq req =
+ SchemaUtils.rollbackUpdateTableReq(database, table.getTableName(), tableName);
+ final boolean proceeded =
+ new ClusterCachePropagator(env.getConfigManager())
+ .propagate(targets -> SchemaUtils.broadcastTableUpdate(req, targets));
+
+ if (!proceeded) {
LOGGER.warn(
ProcedureMessages.FAILED_TO_ROLLBACK_PRE_RELEASE_FOR_TABLE_INFO_TO_DATANODE,
getActionMessage(),
database,
table.getTableName(),
- failedResults);
+ "an unreachable DataNode is not provably fenced");
Review Comment:
follow the i18n standarn, define a constant in both zh and en.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/table/CreateTableProcedure.java:
##########
@@ -151,16 +153,22 @@ protected void preCreateTable(final ConfigNodeProcedureEnv env) {
}
private void preReleaseTable(final ConfigNodeProcedureEnv env) {
- final Map<Integer, TSStatus> failedResults =
- SchemaUtils.preReleaseTable(database, table, env.getConfigManager(), null);
-
- if (!failedResults.isEmpty()) {
- // All dataNodes must clear the related schema cache
+ // Broadcast the pre-update to all DataNodes. Instead of failing whenever any DataNode is
+ // unreachable, proceed once every unacked DataNode is provably self-fenced: such a DataNode
+ // fails closed on its (now-stale) table cache and resyncs on lease recovery, so it cannot serve
+ // dirty schema. Only fail if an unacked DataNode is not provably fenced (it may still be
+ // serving clients).
+ final TUpdateTableReq req = SchemaUtils.BuildPreUpdateTableReq(database, table, null);
+ final boolean proceeded =
+ new ClusterCachePropagator(env.getConfigManager())
+ .propagate(targets -> SchemaUtils.broadcastTableUpdate(req, targets));
+
+ if (!proceeded) {
LOGGER.warn(
ProcedureMessages.FAILED_TO_SYNC_TABLE_PRE_CREATE_INFO_TO_DATANODE_FAILURE,
database,
table.getTableName(),
- failedResults);
+ "an unreachable DataNode is not provably fenced");
Review Comment:
i18n
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/AlterTimeSeriesDataTypeProcedure.java:
##########
@@ -249,26 +246,17 @@ public static void invalidateCache(
final String requestMessage,
final Consumer<ProcedureException> setFailure,
final boolean needLock) {
- final Map<Integer, TDataNodeLocation> dataNodeLocationMap =
- env.getConfigManager().getNodeManager().getRegisteredDataNodeLocations();
- final DataNodeAsyncRequestContext<TInvalidateMatchedSchemaCacheReq, TSStatus> clientHandler =
- new DataNodeAsyncRequestContext<>(
- CnToDnAsyncRequestType.INVALIDATE_MATCHED_SCHEMA_CACHE,
- new TInvalidateMatchedSchemaCacheReq(measurementPathBytes).setNeedLock(needLock),
- dataNodeLocationMap);
- CnToDnInternalServiceAsyncRequestManager.getInstance().sendAsyncRequestWithRetry(clientHandler);
- final Map<Integer, TSStatus> statusMap = clientHandler.getResponseMap();
- for (final TSStatus status : statusMap.values()) {
+ // Proceed past provably-fenced DataNodes instead of hard-failing on the first unreachable one
+ // (see SchemaUtils.invalidateMatchedSchemaCache). Runs before the physical datatype change, so
+ // the "alter only after PROCEED" ordering holds.
+ if (!SchemaUtils.invalidateMatchedSchemaCache(
+ env.getConfigManager(), measurementPathBytes, needLock)) {
// All dataNodes must clear the related schemaEngine cache
- if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- LOGGER.error(
- ProcedureMessages.FAILED_TO_INVALIDATE_SCHEMAENGINE_CACHE_OF_TIMESERIES,
- requestMessage);
- setFailure.accept(
- new ProcedureException(
- new MetadataException(ProcedureMessages.INVALIDATE_SCHEMAENGINE_CACHE_FAILED)));
- return;
- }
+ LOGGER.error(
Review Comment:
warn
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteLogicalViewProcedure.java:
##########
@@ -167,26 +164,15 @@ protected List<TConsensusGroupId> processResponseOfOneDataNode(
}
private void invalidateCache(final ConfigNodeProcedureEnv env) {
- final Map<Integer, TDataNodeLocation> dataNodeLocationMap =
- env.getConfigManager().getNodeManager().getRegisteredDataNodeLocations();
- final DataNodeAsyncRequestContext<TInvalidateMatchedSchemaCacheReq, TSStatus> clientHandler =
- new DataNodeAsyncRequestContext<>(
- CnToDnAsyncRequestType.INVALIDATE_MATCHED_SCHEMA_CACHE,
- new TInvalidateMatchedSchemaCacheReq(patternTreeBytes),
- dataNodeLocationMap);
- CnToDnInternalServiceAsyncRequestManager.getInstance().sendAsyncRequestWithRetry(clientHandler);
- final Map<Integer, TSStatus> statusMap = clientHandler.getResponseMap();
- for (final TSStatus status : statusMap.values()) {
+ if (!SchemaUtils.invalidateMatchedSchemaCache(
+ env.getConfigManager(), patternTreeBytes, false)) {
// all dataNodes must clear the related schemaengine cache
- if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- LOGGER.error(
- ProcedureMessages.FAILED_TO_INVALIDATE_SCHEMAENGINE_CACHE_OF_VIEW, requestMessage);
- setFailure(
- new ProcedureException(
- new MetadataException(
- ProcedureMessages.INVALIDATE_VIEW_SCHEMAENGINE_CACHE_FAILED)));
- return;
- }
+ LOGGER.error(
Review Comment:
warn
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/table/DropTableColumnProcedure.java:
##########
@@ -150,43 +151,48 @@ private void checkAndPreDeleteColumn(final ConfigNodeProcedureEnv env) {
}
private void invalidateCache(final ConfigNodeProcedureEnv env) {
- final Map<Integer, TDataNodeLocation> dataNodeLocationMap =
- env.getConfigManager().getNodeManager().getRegisteredDataNodeLocations();
- final DataNodeAsyncRequestContext<TInvalidateColumnCacheReq, TSStatus> clientHandler =
- new DataNodeAsyncRequestContext<>(
- CnToDnAsyncRequestType.INVALIDATE_COLUMN_CACHE,
- new TInvalidateColumnCacheReq(database, tableName, columnName, isAttributeColumn),
- dataNodeLocationMap);
- CnToDnInternalServiceAsyncRequestManager.getInstance().sendAsyncRequestWithRetry(clientHandler);
- final Map<Integer, TSStatus> statusMap = clientHandler.getResponseMap();
- for (final TSStatus status : statusMap.values()) {
- // All dataNodes must clear the related schemaEngine cache
- if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- LOGGER.error(
- ProcedureMessages.FAILED_TO_INVALIDATE_COLUMN_S_CACHE_OF_TABLE,
- isAttributeColumn ? "attribute" : "measurement",
- columnName,
- database,
- tableName);
- setFailure(
- new ProcedureException(
- new MetadataException(
- String.format(
- ProcedureMessages.INVALIDATE_COLUMN_CACHE_FAILED_FOR_TABLE,
- columnName,
- database,
- tableName))));
- return;
- }
- }
+ TInvalidateColumnCacheReq req =
+ new TInvalidateColumnCacheReq(database, tableName, columnName, isAttributeColumn);
+ final boolean proceeded =
+ new ClusterCachePropagator(env.getConfigManager())
+ .propagate(targets -> broadCastInvalidateCache(req, targets));
+ if (!proceeded) {
+ LOGGER.error(
Review Comment:
no need to be error, just warn is enough, error only for unexpected fatal cases.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/schema/table/PreDeleteTsTable.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.commons.schema.table;
+
+import org.apache.tsfile.utils.ReadWriteIOUtils;
+
+import java.io.IOException;
+import java.io.OutputStream;
+
+public class PreDeleteTsTable extends TsTable {
+ public static final int PRE_DELETE_MARKER = -2;
Review Comment:
```suggestion
public static final int PRE_DELETE_MARKER = -3;
```
put it as an enum, add comments that -2 is reserved and don't use.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/AlterLogicalViewProcedure.java:
##########
@@ -123,27 +120,15 @@ protected Flow executeFromState(
}
private void invalidateCache(final ConfigNodeProcedureEnv env) {
- final Map<Integer, TDataNodeLocation> dataNodeLocationMap =
- env.getConfigManager().getNodeManager().getRegisteredDataNodeLocations();
- final DataNodeAsyncRequestContext<TInvalidateMatchedSchemaCacheReq, TSStatus> clientHandler =
- new DataNodeAsyncRequestContext<>(
- CnToDnAsyncRequestType.INVALIDATE_MATCHED_SCHEMA_CACHE,
- new TInvalidateMatchedSchemaCacheReq(patternTreeBytes),
- dataNodeLocationMap);
- CnToDnInternalServiceAsyncRequestManager.getInstance().sendAsyncRequestWithRetry(clientHandler);
- final Map<Integer, TSStatus> statusMap = clientHandler.getResponseMap();
- for (final TSStatus status : statusMap.values()) {
+ if (!SchemaUtils.invalidateMatchedSchemaCache(
+ env.getConfigManager(), patternTreeBytes, false)) {
// all dataNodes must clear the related schemaengine cache
- if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- LOGGER.error(
- ProcedureMessages.FAILED_TO_INVALIDATE_SCHEMAENGINE_CACHE_OF_VIEW,
- viewPathToSourceMap.keySet());
- setFailure(
- new ProcedureException(
- new MetadataException(
- ProcedureMessages.INVALIDATE_VIEW_SCHEMAENGINE_CACHE_FAILED)));
- return;
- }
+ LOGGER.error(
Review Comment:
warn?
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteTimeSeriesProcedure.java:
##########
@@ -197,26 +194,18 @@ public static void invalidateCache(
final String requestMessage,
final Consumer<ProcedureException> setFailure,
final boolean needLock) {
- final Map<Integer, TDataNodeLocation> dataNodeLocationMap =
- env.getConfigManager().getNodeManager().getRegisteredDataNodeLocations();
- final DataNodeAsyncRequestContext<TInvalidateMatchedSchemaCacheReq, TSStatus> clientHandler =
- new DataNodeAsyncRequestContext<>(
- CnToDnAsyncRequestType.INVALIDATE_MATCHED_SCHEMA_CACHE,
- new TInvalidateMatchedSchemaCacheReq(patternTreeBytes).setNeedLock(needLock),
- dataNodeLocationMap);
- CnToDnInternalServiceAsyncRequestManager.getInstance().sendAsyncRequestWithRetry(clientHandler);
- final Map<Integer, TSStatus> statusMap = clientHandler.getResponseMap();
- for (final TSStatus status : statusMap.values()) {
+ // Proceed once every unreachable DataNode is provably self-fenced (it fails closed on its
+ // schema cache and resyncs on recovery, so it cannot serve the to-be-deleted/altered series),
+ // instead of hard-failing on the first unreachable DataNode. This runs before the physical
+ // delete in the state machine, so the "delete only after PROCEED" ordering holds.
+ if (!SchemaUtils.invalidateMatchedSchemaCache(
+ env.getConfigManager(), patternTreeBytes, needLock)) {
// All dataNodes must clear the related schemaEngine cache
- if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- LOGGER.error(
- ProcedureMessages.FAILED_TO_INVALIDATE_SCHEMAENGINE_CACHE_OF_TIMESERIES,
- requestMessage);
- setFailure.accept(
- new ProcedureException(
- new MetadataException(ProcedureMessages.INVALIDATE_SCHEMAENGINE_CACHE_FAILED)));
- return;
- }
+ LOGGER.error(
Review Comment:
warn
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Cyb/metadata table [iotdb]
Posted by "Caideyipi (via GitHub)" <gi...@apache.org>.
Caideyipi commented on PR #18127:
URL: https://github.com/apache/iotdb/pull/18127#issuecomment-4901444079
I reviewed this PR against the HA requirement/design docs. The direction looks good, but I found several issues that should be addressed before merge:
1. The DataNode fencing decision is not checked synchronously on access.
`MetadataLeaseManager.isFenced()` only checks whether `metadataState != NORMAL`. Lease expiration is moved from `NORMAL` to `NEED_CLEAR` only by the scheduled `checkLeaseStatus()` task. If `check_dn_lease_status_interval_ms` is configured larger than the ConfigNode-side `T_fence + margin`, the ConfigNode may proceed with a metadata change while the partitioned DataNode is still in `NORMAL` and can keep serving stale cache. Please make `isFenced()` / `failIfMetadataLeaseFenced()` check the elapsed heartbeat age and CAS the state to `NEED_CLEAR` immediately when the lease is expired.
2. A reachable but already-fenced DataNode can make the HA broadcast fail.
`DataNodeInternalRPCServiceImpl.updateTable()` calls `DataNodeTableCache.preUpdateTable()`, which throws `MetadataLeaseFencedException` while the DataNode is fenced. On the ConfigNode side, `ClusterCachePropagator` treats any response other than `SUCCESS_STATUS` and `CAN_NOT_CONNECT_DATANODE` as `FAIL`. This means a DataNode that has correctly self-fenced but is still reachable, or is recovering in `NEED_CLEAR` / `PULLING`, can fail the DDL instead of being considered safe. Please return/handle `METADATA_LEASE_FENCED` explicitly and let the propagator treat it as a safe fenced response.
3. The advertised `supportsFencing` compatibility guard is missing.
The design says the ConfigNode should track whether each DataNode supports self-fencing, with default `false` for old nodes. Current `ClusterCachePropagator` only uses heartbeat age. During rolling upgrade or mixed-version operation, an old DataNode that does not self-fence can still be considered safe after `T_proceed`, which is unsafe. Please add capability reporting/tracking and only skip unreachable DataNodes that are known to support fencing.
4. The `fetchTables` Thrift change is not backward compatible.
`fetchTables(map<string, set<string>> fetchTableMap)` was changed to `fetchTables(map<string, set<string>> fetchTableMap, byte tableNodeStatus)`. This breaks old DataNode/new ConfigNode compatibility: an old caller will not set the new argument, and the server can parse it as the default byte value `0` (`PRE_CREATE`), which then goes into the unsupported branch in `ConfigManager.fetchTables()`. Please use a new RPC or an optional/defaulted argument that preserves the old behavior as `USING`.
5. There are new raw English log/message literals in main code.
For example, `ConfigNodeRPCServiceProcessor.reloadCacheAfterLeaseRecovery()` logs `Execute getMetaDataCache with result {}` directly. This repo requires user/operator visible strings to go through the i18n message constants. Please move the new literals to the proper message classes.
Overall, the main concept is sound, but the above issues affect the safety and compatibility of the lease-based HA mechanism.
--
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: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org