Due to database collisions, retry when finding or creating a tag

[fearenales]

This PR closes #693

Since the link posted by @adam-e-trepanier is returning 404, I just implemented a retrial strategy described by Mark Quezada in this blog post.

Any suggestions are welcome! :)

[rbritom]

@fearenales Ill add a test to this

[seuros]

@rbritom Please add a reference in the Changelog too .

[jdelStrother]

I think this approach causes silent data loss.

If you have two simultaneous processes which call TaggableModel.create!(tag_list: "hashtag"), afterwards you'd expect to have 2 TaggableModels stored in the database.
If one of those hits the rollback logic introduced in this PR, you've rolled back the INSERT INTO taggable_models, and nothing outside of the retry-loop knows anything about it. You're left with 1 TaggableModel and no indication that anything went wrong.

@fearenales WDYT?

[gmcnaughton]

I share @jdelStrother 's concern about data loss.

The ROLLBACK here will roll back whatever transaction you're in, undoing any changes made to the db during that transaction. For example:

transaction do
  u = User.create(...)
  u.tag_list.add("foo") # <= a collision here would undo the User.create
end

Out of curiosity, what's the reason for the ROLLBACK?

[jakswa]

Very surprising to me that this ROLLBACK is still in this gem since 2018, with multiple concerns/issues raised over the years. I'm thinking now that I gotta hunt for a way to remove this gem or replace it with another 😩.

[seuros]

@jakswa or open a PR to fix it.

[jakswa]

@jakswa or open a PR to fix it.

@seuros it is open #1081

[seuros]

@jakswa great.
Thanks. I will check it asap.

[jakswa]

Thank you. Sorry if my original comment came off too negative -- we have been troubleshooting borked transactions that led us to this ROLLBACK throwing away data sometimes (but reliably/persistent). If you have two new records getting tagged the same, new tag, you are risking one of those records being ROLLBACK'd silently :-\