CRZbulabula commented on PR #17935: URL: https://github.com/apache/iotdb/pull/17935#issuecomment-4707608313
Good catch — fixed in ddeb9f8fd1. You're right: `SchemaRegionStateMachine.loadSnapshot(...)` returned `true` unconditionally because the underlying load swallowed failures. Both `SchemaRegionPBTreeImpl.loadSnapshot(...)` (catching `IOException | MetadataException`) and `SchemaRegionMemoryImpl.loadSnapshot(...)` (catching `Exception`) logged `FAILED_TO_LOAD_SNAPSHOT`, fell back to `init()`, and returned `void`, so a failed schema-snapshot load was reported as success — weakening the new `IStateMachine.loadSnapshot` contract and, as you note, allowing the Ratis path to advance `lastAppliedTermIndex` after a rejected snapshot. I made the schema-region load path report success/failure: - `ISchemaRegion.loadSnapshot(...)` now returns `boolean` (`false` when the load failed and the region fell back to an empty re-initialized state). - Both `SchemaRegionPBTreeImpl` and `SchemaRegionMemoryImpl` return `true` on the success path and `false` from the fallback/catch path. - `SchemaRegionStateMachine.loadSnapshot(...)` propagates the data-load result, while keeping the pipe-listener recomputation (`schemaListener(...).loadSnapshot` + `listen2Snapshot4PipeListener`) as non-fatal post-processing (consistent with how I scoped the ConfigRegion fix). Added a focused regression test — `SchemaRegionManagementTest#testLoadSnapshotReportsFailureWhenSnapshotIsMissing` — which asserts that loading from a directory without a snapshot reports failure rather than silently succeeding. I also added `assertTrue(...)` to the existing successful-load assertions in `testRatisModeSnapshot`/`testEmptySnapshot`. The new test was verified to fail across all four schema-region modes (MemoryMode, PBTree-Full/Partial/Non-Memory) against the previous always-`true` behavior, and passes with the fix. -- 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]
