Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle potential race condition in which Trac is notified but git fetch doesn't get the changeset #111

Closed
rjollos opened this issue Oct 11, 2016 · 7 comments
Labels

Comments

@rjollos
Copy link
Member

rjollos commented Oct 11, 2016

Discussed near the end of #8, I continue to see cases in which a changeset does not sync from GitHub and therefore the post-commit hook is not triggered. Running a git fetch and trac-admin $env changeset added for the changeset always triggers the update of the relevant ticket.

The following is always seen in the logs when the issue occurs:

2016-07-12 13:07:06,356 Trac[github] ERROR: Payload contains unknown commit 55cd9488ccd897bb9b75450852c100d13cf0df02

I've considered that it might work to implement a simple retry of git remote update --prune.

@aaugustin
Copy link
Contributor

I think I tried that before without success, but perhaps you'll be more persistent and figure it out :-)

@rjollos
Copy link
Member Author

rjollos commented Nov 22, 2016

I see now that the behavior is documented in known issues.

@rjollos
Copy link
Member Author

rjollos commented Mar 6, 2017

6f84cdc has some changes that I'll be testing.

@aaugustin
Copy link
Contributor

The best practice for doing this is exponential backoff.

Start with a given delay (order of magnitude: 0,5 - 1s) and at each retry, multiple the delay by a constant factor (order of magnitude: 1,5 - 2). This is easy to implement and nicer on the remote end if they're struggling.

@neverpanic
Copy link
Member

I'm not sure the re-try is a very good idea; GitHub seems to have pretty strict timeout policy on webhooks (they regularly mark some of ours as "timeout", even though in quite a few of the cases, Trac seems to have processed them).

Maybe we should add a database table, store the webhook and trigger some asynchronous, delayed processing that can then be re-tried until it succeeds?

@rjollos rjollos added the bug label Apr 4, 2017
@rjollos
Copy link
Member Author

rjollos commented Jun 2, 2017

Thanks for the tips. I haven't seen the issue reoccur since this last report. Running my branch with proposed changes, I also never saw log message indicating that the update had to be retried. I'll revisit the issue if it occurs again.

@rjollos rjollos closed this as completed Jun 2, 2017
@stuarta
Copy link

stuarta commented Jun 26, 2018

I'm seeing this approximately every 2 weeks or so with trac 1.2.2.

Just resending the webhook from github doesn't resolve the issue as I still get the "unknown commit"
message in the logs. To resolve I have to manually run git remote update --prune before running a trac-admin <env> repository sync <repo>, after this I can then resend the webhook from github and
trac will get updated.

I will keep trying to understand why we get into this situation.

As per previous comments, I also regularly get timeouts from the github webhooks. Github thinks
they have timed out, but trac does normally get updated. I'm wondering if this is a contributing factor?

I believe we should be accepting the payload from github for async processing (and returning http 202 accepted), then process what github have sent us.

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

No branches or pull requests

4 participants