BookingSync / synced

Keep your BookingSync Application synced with BookingSync
MIT License
3 stars 2 forks source link

Validation failed: Synced has already been taken #79

Open thilonel opened 7 years ago

thilonel commented 7 years ago

This is the error we can see quite often: ActiveRecord::RecordInvalid: Validation failed: Synced has already been taken

I think happens, when there's a new object and two jobs are running in parallel: The first job creates the booking, the second one tries to create the booking as well, and fails with this error. This is the variable that does not reflect the DB state: https://github.com/BookingSync/synced/blob/master/lib/synced/strategies/full.rb#L99 Retrying to save this batch should solve the problem, so that https://github.com/BookingSync/synced/blob/master/lib/synced/strategies/full.rb#L108 won't create a new object, but update the existing one.

WDYT?

Azdaroth commented 7 years ago

@thilonel Maybe it would make sense to pass error handler as a lambda? so that such behaviour could be customised by the consumer.

thilonel commented 7 years ago

What would the user want to customise here? I know I just want the objects to be up to date as soon as possible.

Mandaryn commented 7 years ago

if the problem is from performing doubled syncs at the same time, then preventing doubled syncs at the same time should be a solution, rather than retrying?

Azdaroth commented 7 years ago

@Mandaryn Yeah, that's kinda obvious, but apparently sidekiq unique jobs doesn't work at all. We could try sidekiq enterprise though.

Mandaryn commented 7 years ago

how come it doesn't work, haven't see those errors in b.com and notifs for a long time

Azdaroth commented 7 years ago

@Mandaryn In HA-PM it's almost never worked... I mean of the time it does, but sometimes it doesn't. Seems like there is a bug there, but no idea how to reproduce it.

Fredar commented 7 years ago

are you using until_executed strategy? Thats the only sane strategy to be used.

Other than that we can get sidekiq-enterprise and get uniqueness built by perham

thilonel commented 7 years ago

@Mandaryn that's a valid point, but what if the jobs aren't the same, they just have an intersection? Like a webhook and a scheduled sync?

Mandaryn commented 7 years ago

@thilonel in my apps webhook trigger the same sync as schedule?

thilonel commented 7 years ago

That's irrelevant, unless we say that synced users are to ensure that all synchronized calls are disjunct.

Mandaryn commented 7 years ago

right, the hook will do no harm to me, its not forcing anything :P, just saying that its possible to manage sync uniqueness from the outside without it

thilonel commented 7 years ago

Webhook was a bad example, it my apps it's the same too, but I have another synchronizer which only updates some of the Account's resources, not all of them and that is triggered after instant bookings.