search

ckdake / setler

Setler lets you use the 'Feature Flags' pattern or add settings to models in Rails
MIT License
84 stars 21 forks source link

Add partial unique index for app wide settings #39

Closed tsubery closed 8 years ago

tsubery commented 8 years ago

This index makes sure there are no two settings record with the same name even if both of them are created concurrently.

ckdake commented 8 years ago

Could you speak a little to the issue here and how you ran into it?

tsubery commented 8 years ago

Sure. I didn't actually run into problems, It's just from looking at the code I realized its not thread-safe.

Like many thread safety issues, it's hard to predict upfront what scenario will actually cause this condition. I can think of a situation where there is a web endpoint that sets a global configuration. If you redeploy the admin section that serves that endpoint and have a few seconds downtime, a frustrated user might click the button that calls the endpoint twice. When the application finally goes live it might handle the same request concurrently. In this situation the database could end up with two records for the same configuration. This can lean to confusing behavior as the second record appears on all_settings and the first appears when just calling the finder with Setting[:setting]. So you can end up with a setting that appears on #all_settings method but cannot be changed with Setting[:setting] = "disable". The root cause of the problem is find_or_create_by method on activerecord is not "atomic". It basically performs two steps of searching and creating. Between those steps another thread/process can also search and create the same record.

There's a good explanation of the problem in the method documentation.

ckdake commented 8 years ago

This does complicate things a little bit, but it does make sense. I suspect this won't happen too often regardless!