Migrate sync flag using state API appears to be broken.

Looks like there was some discussion in 🐛 sync option doesnt work with track_changes Fixed about keying state values with migration names ( #3104105-25: sync option doesnt work with track_changes → ), but the final implementation didn't actually do that.

@phma also noted in #3104105-19: sync option doesnt work with track_changes → that the changes to the sync implementation would likely also break BC for using the sync and update options together which may also be affecting sites having problems with sync.

The test @trackleft2 referenced in the issue summary fails in the Migrate Queue Importer module issue/MR that upgrades Migrate Tools to 6.x ( ✨ Update dependency configuration to support both 6.x and 5.x versions of Migrate Tools and Migrate Plus simultaneously. Fixed ).

The potential ID collision issue raised by @trackleft2 in the issue summary is the thing I'm most concerned about. I don't yet have a test or list of steps to reproduce such a collision but it doesn't seem like the current state API implementation would prevent ID collisions between different migrations.

The attached patch is a suggestion for how these hypothetical collisions could be prevented.

Do you think that we are going to need a database update, or an update process of some kind for this as well? (Even though the current implementation probably doesn't work anyway)

I've added a test to try to prove the issue, but ended up more confused than I started.

Turns out that there is no risk of id collision, because the state is being treated as scratch data?

However I did notice that the data in state is set twice, which is strange.

https://git.drupalcode.org/issue/migrate_tools-3347272/-/compare/6.0.x.....

Is it possible that the state is being updated upon PreMigrateImport and PreMigrateRowImport causing duplication of state data?

I don't know if this is related, but I'm having a strange problem with --sync:

• If I run drush mim --all --sync, certain records that are supposed to be deleted by --sync remain.
• If I run the migration individually with drush mim MIGRATION-ID --sync, those records are deleted as expected.

Did you try that patch @jigarius?