Skip to content

Fix schema region NPE during DataNode shutdown - #18821

Open
jt2594838 wants to merge 2 commits into
masterfrom
fix_stop_schema_region_npe
Open

jt2594838 wants to merge 2 commits into
masterfrom
fix_stop_schema_region_npe

Conversation

@jt2594838

@jt2594838 jt2594838 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Description

During DataNode shutdown, SchemaEngine can clear a schema region while its validation lock is still registered. An in-flight schema write request then dereferences the missing region and returns INTERNAL_SERVER_ERROR (305).

Check region availability before accessing the schema region and return the retryable NO_AVAILABLE_REGION_GROUP (906) status when shutdown has already cleared it. The original fix covers internal timeseries creation quota validation. The same guard is applied to the highly similar AlterTimeSeries validation, Ratis CreateLogicalView validation, and CreateOrUpdateTableDevice quota validation paths. Existing validation and consensus writes remain unchanged when the region is available; SimpleConsensus logical-view creation retains its direct consensus-write behavior.

Validation

  • Reproduced the original single-device and batch failures before the fix: expected status 906, received 305.
  • Reproduced the three similar missing-region paths before the fix: expected status 906, received 305.
  • mvn spotless:apply test -pl iotdb-core/datanode -am -Drat.skip=true -Dtest=RegionWriteExecutorTest -Dsurefire.failIfNoSpecifiedTests=false: passed, 7 tests with no failures, errors, or skips. Tests cover internal timeseries creation, AlterTimeSeries, CreateLogicalView, CreateOrUpdateTableDevice, ordinary/Pipe requests, Ratis/SimpleConsensus, and available/missing regions.
  • Full DataNode module build: passed.
  • git diff --check: passed. No full cluster shutdown stress test was performed.

This PR has:

  • been self-reviewed.
  • added comments explaining the shutdown lifecycle constraint.
  • updated the prevalidation method documentation.
  • added unit tests covering the changed behavior.

Key changed classes: RegionWriteExecutor and RegionWriteExecutorTest, plus the locale message changes from the original fix.


public final class DataNodeQueryMessages {
public static final String MESSAGE_SCHEMA_REGION_IS_UNAVAILABLE_PLEASE_RETRY_LATER_D642510A =
"Schema region is unavailable. Please retry later.";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adds the retryable missing-region message to the English message catalog so the new validation result participates in the existing compile-time localization mechanism.


public final class DataNodeQueryMessages {
public static final String MESSAGE_SCHEMA_REGION_IS_UNAVAILABLE_PLEASE_RETRY_LATER_D642510A =
"SchemaRegion 不可用,请稍后重试。";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adds the same message key in Chinese to preserve locale parity. DataNode and ConfigNode compiled successfully with the Chinese profile; the full reactor stopped later at distribution because SNAPSHOT ZIP dependencies were unavailable.

final String message =
DataNodeQueryMessages.MESSAGE_SCHEMA_REGION_IS_UNAVAILABLE_PLEASE_RETRY_LATER_D642510A;
return RegionExecutionResult.create(
false, message, RpcUtils.getStatus(TSStatusCode.NO_AVAILABLE_REGION_GROUP, message));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Shutdown can clear the schema region while its validation lock is still registered. The previous quota check dereferenced null, and the outer catch returned INTERNAL_SERVER_ERROR because the lock still existed. Checking availability here returns NO_AVAILABLE_REGION_GROUP before dereferencing the region and covers the shared timeseries prevalidation callers. The method documentation now describes all three checks. Existing live-region validation and consensus writes are preserved.

@Test
public void testInternalCreateTimeSeriesAfterSchemaRegionCleared() throws Exception {
// A shutdown can clear the region while its validation lock remains registered.
checkInternalCreateWithRegionAvailability(false, false);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reproduces the single-device internal creation failure by retaining the validation lock while returning a missing region. Before the fix this test received status 305 instead of the retryable 906; it now passes.

@Test
public void testInternalCreateMultiTimeSeriesAfterSchemaRegionCleared() throws Exception {
// Batch and Pipe auto-creation must return a retryable status for the same shutdown window.
checkInternalCreateWithRegionAvailability(true, false);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Covers batch internal creation against the same missing-region state so both auto-creation entry points have regression coverage. The helper also exercises Pipe-wrapped requests.

public void testInternalCreateWithAvailableSchemaRegion() throws Exception {
// The guard must preserve quota checks and consensus writes for both live-region requests.
checkInternalCreateWithRegionAvailability(false, true);
checkInternalCreateWithRegionAvailability(true, true);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Checks both single-device and batch requests with an available region to ensure that the early rejection does not suppress valid quota checks or consensus writes.

} finally {
IoTDBDescriptor.getInstance()
.getConfig()
.setSchemaRegionConsensusProtocolClass(originalProtocol);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The shared fixture runs SimpleConsensus and Ratis with ordinary and Pipe requests, using a real validation lock and mocked region/consensus services. Assertions check the returned status, lock release, and either the expected write or the absence of consensus interaction; configuration is restored in finally. All four tests in this class passed. This is deterministic executor-level coverage, not a full cluster shutdown stress test. The added imports support this fixture and its request variants.

// Shutdown may clear SchemaEngine while the region's validation lock still exists.
// Reject the request as retryable before dereferencing the missing region.
if (schemaRegion == null) {
return createSchemaRegionUnavailableResult();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This reuses the centralized retryable result for internal timeseries creation after the original guard. Keeping one result builder ensures shutdown races consistently return NO_AVAILABLE_REGION_GROUP (906) with the localized message across all schema-write prechecks; live-region quota and type validation remain unchanged.

AlterTimeSeriesNode node, WritePlanNodeExecutionContext context, boolean receivedFromPipe) {
ISchemaRegion schemaRegion =
schemaEngine.getSchemaRegion((SchemaRegionId) context.getRegionId());
if (schemaRegion == null) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

AlterTimeSeries fetches its measurement before consensus, so a cleared SchemaEngine entry could otherwise be dereferenced while the validation lock still exists. Returning the same retryable 906 result here preserves the shutdown behavior established for timeseries creation; the available-region path remains unchanged.

final ISchemaRegion schemaRegion =
schemaEngine.getSchemaRegion((SchemaRegionId) context.getRegionId());
if (CONFIG.getSchemaRegionConsensusProtocolClass().equals(ConsensusFactory.RATIS_CONSENSUS)) {
if (schemaRegion == null) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ratis validates logical-view targets locally before writing consensus state. Guarding the missing region prevents a shutdown-time null dereference and makes the request retryable; SimpleConsensus continues to use its existing direct write path.

final boolean receivedFromPipe) {
final ISchemaRegion schemaRegion =
schemaEngine.getSchemaRegion((SchemaRegionId) context.getRegionId());
if (schemaRegion == null) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Table-device quota validation also reads the schema region before consensus. This guard covers the same shutdown window as the timeseries path and returns 906 before checkSchemaQuota is called, while preserving normal validation for an available region.

: PlanVisitor.super.visitCreateOrUpdateTableDevice(node, context);
}

private RegionExecutionResult createSchemaRegionUnavailableResult() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Centralizing the unavailable-region result keeps all guarded schema-write paths aligned on the retryable status and localized message. The helper also documents why a still-registered validation lock must not be treated as proof that the region is usable.

public class RegionWriteExecutorTest {

@Test
public void testAlterTimeSeriesRegionAvailability() throws Exception {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This regression test exercises AlterTimeSeries with the validation lock retained while the schema region is absent. It verifies the new retryable 906 result and the unchanged successful path for ordinary and Pipe requests under both consensus protocols.

}

@Test
public void testCreateLogicalViewRegionAvailability() throws Exception {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This test covers both logical-view consensus modes: Ratis performs local target validation and must reject a missing region with 906, while SimpleConsensus keeps its direct write behavior. The matrix also checks available regions, Pipe wrapping, and lock cleanup.

}

@Test
public void testCreateOrUpdateTableDeviceRegionAvailability() throws Exception {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This regression test covers table-device quota validation during the same region-cleared window. It verifies retryable failure without consensus writes, plus successful quota validation and writes when the region is available.

Collections.singletonList(new Object[0])));
}

private void checkSchemaWriteWithRegionAvailability(final WritePlanNode node) throws Exception {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The shared test matrix keeps the similar-path checks consistent across missing and available regions, both consensus protocols, and ordinary/Pipe requests. It also verifies the validation lock is released and that consensus is skipped only for the unavailable-region case.

@codecov

codecov Bot commented Oct 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.88%. Comparing base (40e7ce6) to head (df08230).
⚠️ Report is 21 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18821      +/-   ##
============================================
+ Coverage     45.57%   45.88%   +0.31%     
  Complexity      712      712              
============================================
  Files          5483     5498      +15     
  Lines        396291   397656    +1365     
  Branches      51567    51773     +206     
============================================
+ Hits         180590   182455    +1865     
+ Misses       215701   215201     -500     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant