-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Commit build status not updating (intermittent) #14059
Comments
I've managed to recreate this on my dev instance using a script to fire simultaneous commit updates at the API. Interestingly it didn't trigger the error when using sqlite3, however when I switch to mariadb10.3 it happens. I used bash script:
Which gives:
I'm still thinking a lock on the db table would solve this. Or maybe the index field could be auto increment? I'm not sure why there is an index field? Looking at the xorm docs it looks like |
So with
I guess I could catch that error and recall the NewCommitStatus function? Though why not just catch duplicate entry error and retry. It doens't look like Not sure I know what I'm doing enough to solve this one. Maybe using an autoincrement index may be better? |
It looks like the If this is the case, then we could do this using the We could remove the |
|
OK, so So I think maybe the solution is to check for How would I test for that error, |
But this is only for mysql, for other database it will return different error. Maybe we could just retry serval times one returned error as a temporary solution. |
OK. I think I would be able to make a retry loop, say 3 to 5 times perhaps? maybe with a couple hundred miliseconds delay between retries? Though as you say it feels like a temporary workaround. I'm tempted to remove that |
The index column could be used to know the sequence for commit statuses of one commit. Maybe we should not remove it. |
Yes. But the same sequence can also be known by ordering by the
If this is its only use then I could refactor to use |
If there is another reason for the index field then this of course is not feasible. It gives me another idea though. We could insert the record with a null index, then select all records for that sha order by id, loop through counting and update any with null index to the correct index count. Again seems a bit of a kludge. There is a possibility the table will be queried from elsewhere while a row has a null index. |
I've removed index and refactored accordingly, everything builds and drone integration still works fine :) I will create PR so we can review there. |
We are running into this issue as well on our postgres-backed Gitea. Thanks for working on a solution, @bobemoe! For now, our calling script has baked in delayed retries to try and avoid this. Sorting by ID does seem like a good workaround. There's also already timestamps for created/updated which could be guaranteed to be sortable, regardless of the backing database server type. |
[x]
):Description
Occasionally one of my Drone builds will not update it's status properly, leaving the commit as "Build in progress" with the yellow circle, when actually checking Drone it shows "Successful", or "Failed".
This only happens occasionally, the only pattern I can see is its only been for PR's so far, and also when the server is under heavy load. I first noticed when I ran "update branch" on 4 PR's at the same time, each calls 2 builds, so I have many builds queued.
From the logs, the error seems to be a SQL duplicate value in the unique index.
Looking at how the index is generated, I'm guessing two status updates came in simultaneously and both generated the same value? Maybe a lock on the db table is needed during index increment?
gitea/models/commit_status.go
Lines 220 to 240 in 4aabbac
I notice that file has been updated yesterday, but looking at the code it looks like the index is still generated the same way. I will update my gitea anyway and keep an eye on things.Just happened again on latest master. I've updated Gitea version above.The text was updated successfully, but these errors were encountered: