Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Potential race condition causing commits to fail #80

Open
boblauer opened this issue Jul 31, 2017 · 2 comments
Open

Potential race condition causing commits to fail #80

boblauer opened this issue Jul 31, 2017 · 2 comments

Comments

@boblauer
Copy link

boblauer commented Jul 31, 2017

I don't know Ruby, and I'm not entirely familiar with how everything in this app works, so this is a bit of a shot in the dark.

I will use a simplified version of my current setup to try to describe what I suspect is happening.

I have 3 files, en.pot, es.pot, and fr.pot.

  • I make a change to en.pot and commit/push directly to master
  • That updated file is sent to Transifex via Txgh.
  • es.pot and fr.pot are updated in Transifex to have this new entry
  • Transifex fires off 2 webhook requests to Txgh, one for es.pot and one for fr.pot, both very close to each other in terms of timing
  • Txgh creates a new commit for es.pot, using the current base_commit as the parent commit.
  • At roughly the same time, Txgh creates a new commit for fr.pot, also using this same base_commit as the parent commit.
  • The first commit is successfully pushed to master, but the 2nd commit fails with the error:
- Update is not a fast forward // See: https://developer.github.com/v3/git/refs/#update-a-reference:

To debug this a bit further, I modified the source code to enable force pushing. With force pushing turned on, I was able to see a commit from Txgh come through (in our example, it would be es.pot being updated), and then that commit was wiped from the repo's history a moment later when fr.pot was updated, meaning fr.pot was a force push.

I don't know how to fix this behavior (nor am I 100% certain I am right, although I am confident I'm at least on the right path). The only suggestion off the top of my head is switching to using this much simpler api for updating a file - https://developer.github.com/v3/repos/contents/#update-a-file

Any suggestions on possible work-around for this?

@boblauer
Copy link
Author

boblauer commented Aug 2, 2017

We were able to work around this by adding a sleep to the beginning of the Transifex handler's execute method that increased with each subsequent request. This allowed us to effectively stagger these requests and avoid a race condition. Not ideal, but it works for us.

Since the problem has not been solved, but only worked around, I will leave this issue open, in case you want to investigate further and apply a proper fix, if necessary.

@matthewjackowski
Copy link
Contributor

@boblauer I agree that both solutions ("forcing" or "sleeping") are likely less than ideal.

My thought is that the async / stateless nature of webhook -> sinatra is actually the root problem.

Another way to solve this would be to add a configuration/mode that only allows one execute to run at a time. That way this GitHub requests would always be handled sequentially. However this config would take a bit of work because in addition to making the service a little-bit-stateful, we would also need to queue incoming webhook requests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants