Caideyipi commented on code in PR #18265:
URL: https://github.com/apache/iotdb/pull/18265#discussion_r3622283145
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/ClusterTemplateManager.java:
##########
@@ -625,6 +644,7 @@ public void updateTemplateInfo(byte[] templateInfo) {
public void updateTemplateInfo(Template template) {
readWriteLock.writeLock().lock();
try {
+ failIfMetadataLeaseFenced();
Review Comment:
Template propagation can fail during the asynchronous lease-recovery window.
The heartbeat schedules cache recovery asynchronously, so the ConfigNode may
already consider a DataNode reachable while its local metadata state is still
fenced. A template update then throws from `failIfMetadataLeaseFenced()`.
The added regression test reproduces the actual RPC path: the Thrift
application exception is converted by `DataNodeTSStatusRPCHandler` to
EXECUTE_STATEMENT_ERROR (301), after which `ClusterCachePropagator` returns
FAIL instead of WAIT.
This causes SET/UNSET/ALTER to fail transiently while the DataNode is
recovering. Please preserve/classify the fenced condition as retryable and
retry until cache recovery completes.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java:
##########
@@ -3308,7 +3308,8 @@ public TDataNodeLeaseRecoveryResp
reloadCacheAfterLeaseRecovery() {
}
return new TDataNodeLeaseRecoveryResp()
.setStatus(RpcUtils.SUCCESS_STATUS)
-
.setTableInfo(clusterSchemaManager.getAllTableInfoForDataNodeActivation());
+
.setTableInfo(clusterSchemaManager.getAllTableInfoForDataNodeActivation())
+ .setTemplateInfo(clusterSchemaManager.getAllTemplateSetInfo());
Review Comment:
`reloadCacheAfterLeaseRecovery()` should not return SUCCESS when the
template snapshot cannot be read.
`ClusterSchemaManager#getAllTemplateSetInfo()` catches `ConsensusException`
and returns `new byte[0]`. The recovery RPC then returns SUCCESS with that
empty value. On the DataNode side, the field is considered set, so the template
cache is cleared and the metadata lease returns to NORMAL.
The added regression tests reproduce both steps: an injected ConfigRegion
read failure produces a zero-length template snapshot, and that snapshot still
produces a SUCCESS (code 200) lease-recovery response.
This can unfence a DataNode with all template metadata missing. Please
propagate the snapshot read failure through the RPC status so the DataNode
remains fenced.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/UnsetTemplateProcedure.java:
##########
@@ -154,30 +155,21 @@ private void invalidateCache(final ConfigNodeProcedureEnv
env) {
}
}
- private void executeInvalidateCache(final ConfigNodeProcedureEnv env) throws
ProcedureException {
- final Map<Integer, TDataNodeLocation> dataNodeLocationMap =
-
env.getConfigManager().getNodeManager().getRegisteredDataNodeLocations();
- final TUpdateTemplateReq invalidateTemplateSetInfoReq = new
TUpdateTemplateReq();
- invalidateTemplateSetInfoReq.setType(
- TemplateInternalRPCUpdateType.INVALIDATE_TEMPLATE_SET_INFO.toByte());
-
invalidateTemplateSetInfoReq.setTemplateInfo(getInvalidateTemplateSetInfo());
- final DataNodeAsyncRequestContext<TUpdateTemplateReq, TSStatus>
clientHandler =
- new DataNodeAsyncRequestContext<>(
- CnToDnAsyncRequestType.UPDATE_TEMPLATE,
- invalidateTemplateSetInfoReq,
- dataNodeLocationMap);
-
CnToDnInternalServiceAsyncRequestManager.getInstance().sendAsyncRequestWithRetry(clientHandler);
- final Map<Integer, TSStatus> statusMap = clientHandler.getResponseMap();
- for (final TSStatus status : statusMap.values()) {
- // all dataNodes must clear the related template cache
- if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- LOGGER.error(
-
ProcedureMessages.FAILED_TO_INVALIDATE_TEMPLATE_CACHE_OF_TEMPLATE_SET_ON,
- template.getName(),
- path);
- throw new ProcedureException(
- new
MetadataException(ProcedureMessages.INVALIDATE_TEMPLATE_CACHE_FAILED));
- }
+ void executeInvalidateCache(final ConfigNodeProcedureEnv env) throws
ProcedureException {
+ final TUpdateTemplateReq req = new TUpdateTemplateReq();
+
req.setType(TemplateInternalRPCUpdateType.INVALIDATE_TEMPLATE_SET_INFO.toByte());
+ req.setTemplateInfo(getInvalidateTemplateSetInfo());
+
+ final boolean proceed =
+ new
ClusterCachePropagator(SchemaUtils.filterFencedDataNode(env.getConfigManager()))
Review Comment:
The rollback path is not HA-safe after allowing the forward invalidation to
skip fenced DataNodes.
If UNSET reaches the activation check and finds that the template is still
in use, rollback calls `executeRollbackInvalidateCache()` before
`rollbackPreUnsetSchemaTemplate()`. The former still broadcasts to every
registered DataNode and fails when one DataNode is offline. Because of that
exception, the ConfigNode PRE_UNSET state is never restored.
The added regression test injects the DataNode rollback failure and confirms
that `rollbackPreUnsetSchemaTemplate()` is called zero times.
Please restore the authoritative ConfigNode state independently of DataNode
acknowledgements, and then use fence-aware propagation to repair the caches of
reachable DataNodes.
--
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]
