JackieTien97 commented on code in PR #18265:
URL: https://github.com/apache/iotdb/pull/18265#discussion_r3627176818
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/ClusterTemplateManager.java:
##########
@@ -273,6 +274,7 @@ public List<PartialPath> getPathsSetTemplate(String name,
PathPatternTree scope)
public Template getTemplate(int id) {
readWriteLock.readLock().lock();
try {
+ failIfMetadataLeaseFenced();
Review Comment:
[P1] Keep lease fencing out of SchemaRegion apply
`getTemplate(int)` is also called from
`SchemaExecutionVisitor#visitActivateTemplate` and `visitBatchActivateTemplate`
while applying committed SchemaRegion consensus entries on every replica. A
replica can lose ConfigNode heartbeats and become fenced while it is still
receiving SchemaRegion logs, so this unchecked `MetadataLeaseFencedException`
escapes `SchemaRegionStateMachine#write` and makes apply depend on
replica-local lease state. Please enforce the lease before consensus
submission, but make state-machine apply independent of the fenced request
cache—for example, carry the template definition/version in the plan or use a
non-fenced durable lookup during apply.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/ClusterTemplateManager.java:
##########
@@ -678,11 +699,30 @@ public void putTemplate(Template template) {
}
public void clear() {
- templateIdMap.clear();
- templateNameMap.clear();
- pathSetTemplateMap.clear();
- templateSetOnPathsMap.clear();
- pathPreSetTemplateMap.clear();
- templatePreSetOnPathsMap.clear();
+ readWriteLock.writeLock().lock();
+ try {
+ templateIdMap.clear();
+ templateNameMap.clear();
+ pathSetTemplateMap.clear();
+ templateSetOnPathsMap.clear();
+ pathPreSetTemplateMap.clear();
+ templatePreSetOnPathsMap.clear();
+ } finally {
+ readWriteLock.writeLock().unlock();
+ }
+ }
+
+ public void reloadTemplateCacheAfterLeaseRecovery(byte[] templateSetInfo) {
+ readWriteLock.writeLock().lock();
+ try {
+ clear();
+ initTemplateSetInfo(templateSetInfo);
Review Comment:
[P1] Reconcile active-template counts during recovery
This reload replaces only the template maps. If a template with active
devices is ALTERed while this DataNode is fenced, it misses
`UPDATE_TEMPLATE_INFO`—the only path that computes the old/new measurement
delta and calls `SchemaEngine#updateSubtreeMeasurementCountForTemplate`. After
recovery, Memory SchemaRegions therefore retain the old
`subtreeMeasurementCount` while the template has the new schema, which can make
OFFSET-based traversal prune incorrectly and later deactivation subtract the
wrong count. Please rebuild these derived counts, or apply the per-template
deltas, before returning the lease to NORMAL.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/template/ClusterTemplateManager.java:
##########
@@ -678,11 +699,30 @@ public void putTemplate(Template template) {
}
public void clear() {
- templateIdMap.clear();
- templateNameMap.clear();
- pathSetTemplateMap.clear();
- templateSetOnPathsMap.clear();
- pathPreSetTemplateMap.clear();
- templatePreSetOnPathsMap.clear();
+ readWriteLock.writeLock().lock();
+ try {
+ templateIdMap.clear();
+ templateNameMap.clear();
+ pathSetTemplateMap.clear();
+ templateSetOnPathsMap.clear();
+ pathPreSetTemplateMap.clear();
+ templatePreSetOnPathsMap.clear();
+ } finally {
+ readWriteLock.writeLock().unlock();
+ }
+ }
+
+ public void reloadTemplateCacheAfterLeaseRecovery(byte[] templateSetInfo) {
+ readWriteLock.writeLock().lock();
Review Comment:
[P1] Serialize recovered template sets with in-flight CREATEs
Normal `ADD_TEMPLATE_PRE_SET_INFO` takes the global `TIMESERIES_VS_TEMPLATE`
write lock because a CREATE that passed the analyzer check holds the matching
read lock through SchemaRegion apply. This recovery path installs SET/PRE_SET
entries under only the manager-local lock, so it can publish a template while
such a CREATE is still in flight. SchemaRegion create/apply does not repeat the
template compatibility check, allowing an ordinary measurement to be committed
under the newly set template path. Please take the same global write lock
around the recovered template swap, or provide equivalent serialization.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/schemaengine/lease/MetadataLeaseManager.java:
##########
@@ -222,20 +223,38 @@ private void pullMetaDataAndInit() {
LOGGER.error(DataNodeSchemaMessages.FAILED_TO_MARK_METADATA_STATE_AS_PULLING,
metadataState);
return;
}
-
- for (final MetadataAction action : pullMetaList) {
- try {
- action.execute();
- } catch (final Throwable t) {
- metadataStateRef.set(MetadataState.PULL_OR_INIT_FAILED,
metadataStateRef.getStamp() + 1);
- LOGGER.error(DataNodeSchemaMessages.FAILED_TO_PULL_OR_INIT_METADATA,
t);
- rethrowUnchecked(t);
- }
+ try {
+ reloadRelatedCache();
+ } catch (final Throwable t) {
+ metadataStateRef.set(MetadataState.PULL_OR_INIT_FAILED,
metadataStateRef.getStamp() + 1);
+ LOGGER.error(DataNodeSchemaMessages.FAILED_TO_PULL_OR_INIT_METADATA, t);
+ rethrowUnchecked(t);
}
+
this.lastConfigNodeHeartbeatNanos = nanoClock.getAsLong();
metadataStateRef.set(MetadataState.NORMAL, metadataStateRef.getStamp() +
1);
}
+ void reloadRelatedCache() {
+ try (ConfigNodeClient configNodeClient =
+
ConfigNodeClientManager.getInstance().borrowClient(ConfigNodeInfo.CONFIG_REGION_ID))
{
+
+ final TDataNodeLeaseRecoveryResp resp =
configNodeClient.reloadCacheAfterLeaseRecovery();
Review Comment:
[P1] Add a revision barrier to the recovery snapshot
This RPC returns only a point-in-time snapshot. A concurrent SET/UNSET/ALTER
procedure can select its broadcast targets while this DataNode is still
regarded as fenced, commit after the snapshot was read, and therefore
legitimately omit this node. Recovery then installs the older snapshot and
marks the lease NORMAL, with no revision check or delta catch-up, so the node
can remain permanently stale. Please keep the node fenced until a ConfigRegion
revision covering concurrent mutations has been applied, or otherwise linearize
snapshot generation with update propagation.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java:
##########
@@ -1314,23 +1314,17 @@ public synchronized TSStatus extendSchemaTemplate(
Map<Integer, TDataNodeLocation> dataNodeLocationMap =
configManager.getNodeManager().getRegisteredDataNodeLocations();
- DataNodeAsyncRequestContext<TUpdateTemplateReq, TSStatus> clientHandler =
- new DataNodeAsyncRequestContext<>(
- CnToDnAsyncRequestType.UPDATE_TEMPLATE, updateTemplateReq,
dataNodeLocationMap);
-
CnToDnInternalServiceAsyncRequestManager.getInstance().sendAsyncRequestWithRetry(clientHandler);
- Map<Integer, TSStatus> statusMap = clientHandler.getResponseMap();
- for (Map.Entry<Integer, TSStatus> entry : statusMap.entrySet()) {
- if (entry.getValue().getCode() !=
TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
- LOGGER.warn(
- ManagerMessages.FAILED_TO_SYNC_TEMPLATE_EXTENSION_INFO_TO_DATANODE,
- template.getName(),
- dataNodeLocationMap.get(entry.getKey()));
- return RpcUtils.getStatus(
- TSStatusCode.EXECUTE_STATEMENT_ERROR,
- String.format(
- "Failed to sync template %s extension info to DataNode %s",
- template.getName(), dataNodeLocationMap.get(entry.getKey())));
- }
+ // The template extension is already committed and cannot be rolled back.
+ // Unexpected DataNode internal failures cannot be handled here.
+ final boolean proceed =
+ new ClusterCachePropagator(dataNodeLocationMap)
+ .propagate(targets -> broadcastTemplateUpdate(updateTemplateReq,
targets));
Review Comment:
[P1] Prevent stale ALTER retries after a leader change
This retry keeps sending the serialized full template captured above,
without a ConfigNode term or template version. If this ConfigNode loses
leadership while `propagate` is waiting, the new leader can commit and
broadcast a later extension before a retry from the old leader arrives.
`DataNodeInternalRPCServiceImpl#updateTemplate` then unconditionally replaces
the cached template and applies the measurement-count delta in arrival order,
so the late retry regresses both the schema and subtree counts. Please abort or
reconcile retries on leadership loss, or version these updates so DataNodes
reject older snapshots.
--
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]
