RiversJin commented on PR #2829: URL: https://github.com/apache/kvrocks/pull/2829#issuecomment-2728032273
> `SlotMigrator::syncWALByCmd()` would first sync without forbidden the slot, then it would forbidden the slot and fetch the incremental. `SlotMigrator::finishFailedMigration()` would cleanup the `forbidden_slot_`. What raise my curious is that why it didn't solved in the success or cleanup path? `SlotMigrator::syncWALByCmd()` My colleague speculated that this is to allow GetMigrationInfo to print out the migrated slots after migration. Since after migration, the controller needs to execute setslot to confirm the migration, these forbidden slots could technically be cleared once the migration is confirmed by the controller. However, this approach might be cumbersome that the confirmation could be via setslot or setnodes, and we also need to account for edge cases (e.g., the target shard being deleted?). So from the perspective of the simplest fix, directly clearing the forbidden slot at the import stage during re-migration would also work. -- 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]
