Caideyipi commented on PR #17935:
URL: https://github.com/apache/iotdb/pull/17935#issuecomment-4706867388

   I found one remaining contract gap in the snapshot-load failure propagation.
   
   SchemaRegionStateMachine.loadSnapshot(...) now returns       rue after 
calling schemaRegion.loadSnapshot(...), but the underlying ISchemaRegion 
implementations still swallow load failures:
   
   - SchemaRegionPBTreeImpl.loadSnapshot(...) catches IOException | 
MetadataException, logs FAILED_TO_LOAD_SNAPSHOT, tries init(), and then returns 
normally.
   - SchemaRegionMemoryImpl.loadSnapshot(...) has the same pattern for 
Exception.
   
   Because those methods are still oid and do not rethrow or return a result, 
SchemaRegionStateMachine.loadSnapshot(...) can report success even when the 
schema snapshot was not actually loaded. This weakens the new 
IStateMachine.loadSnapshot boolean contract. It also matters for Ratis, since 
ApplicationStateMachineProxy now relies on a alse return to avoid advancing 
lastAppliedTermIndex after a rejected snapshot.
   
   Suggested fix: make the schema-region load path report success/failure as 
well, or make SchemaRegionStateMachine.loadSnapshot(...) verify that the 
underlying schema snapshot was actually loaded before returning   rue. A 
focused regression test around a failing schema snapshot load would help keep 
this contract from regressing.


-- 
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]

Reply via email to