Skip to content

support the high availability of the device template related procedure#18265

Open
alpass163gmail wants to merge 6 commits into
apache:masterfrom
alpass163gmail:cyb/schem_template_HA
Open

support the high availability of the device template related procedure#18265
alpass163gmail wants to merge 6 commits into
apache:masterfrom
alpass163gmail:cyb/schem_template_HA

Conversation

@alpass163gmail

Copy link
Copy Markdown
Contributor

support the high availability of the device template related procedure

@Caideyipi Caideyipi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added focused regression coverage in commit 4f9f3ef. The four assertions currently fail as described below; five control tests pass.

.setStatus(RpcUtils.SUCCESS_STATUS)
.setTableInfo(clusterSchemaManager.getAllTableInfoForDataNodeActivation());
.setTableInfo(clusterSchemaManager.getAllTableInfoForDataNodeActivation())
.setTemplateInfo(clusterSchemaManager.getAllTemplateSetInfo());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

req.setTemplateInfo(getInvalidateTemplateSetInfo());

final boolean proceed =
new ClusterCachePropagator(SchemaUtils.filterFencedDataNode(env.getConfigManager()))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

public void updateTemplateInfo(Template template) {
readWriteLock.writeLock().lock();
try {
failIfMetadataLeaseFenced();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@JackieTien97 JackieTien97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Five additional HA correctness issues found on the current head.

public Template getTemplate(int id) {
readWriteLock.readLock().lock();
try {
failIfMetadataLeaseFenced();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

readWriteLock.writeLock().lock();
try {
clear();
initTemplateSetInfo(templateSetInfo);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

}

public void reloadTemplateCacheAfterLeaseRecovery(byte[] templateSetInfo) {
readWriteLock.writeLock().lock();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

// Unexpected DataNode internal failures cannot be handled here.
final boolean proceed =
new ClusterCachePropagator(dataNodeLocationMap)
.propagate(targets -> broadcastTemplateUpdate(updateTemplateReq, targets));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants