#1726 Make updating the git repos a locked process
Merged by pingou. Opened by pingou.
lock_git  into  master

Download 1726.patch
no initial comment

1 new commit added

  • Catch the timeout exception filelock will raise if it takes too long

I think this should help preventing running into https://pagure.io/pagure/issue/1445 again

Shouldn't this also be indented?

Do we really want to do this? This might kill other threads that were waiting for the lock file. I think that lockfile should release it when it leaves the context, not?

Good catch, it's defined in the with section, fixing in both places

Hm, indeed I don't know how it will do with other threads waiting on it, but it's not removed automatically: https://github.com/benediktschmitt/py-filelock/blob/master/filelock.py#L280 and I thought it would be nicer if we could remove it so that when checking the FS we know which are being used (or that we have left over)

From a simple test with two scripts running in two terminals, it seems be behave fine

2 new commits added

  • Catch in more places the potential filelock.Timeout exception
  • Fix indentation for removing the fork inside the block where it is defined

This PR looks good till now , questions I wanted to ask are:
1. Did we have any race conditions happening ?
2. How will file lock impact the efficiency of pagure ?

I don't understand file lock to deeper extent but I tried to look into the code. :smile:

We have had ticket git repo with orphan/lost commit because something happen when pushing the commit which I think was due to some race-condition.

See as ticket https://pagure.io/pagure/issue/1445

The lock will definitely impact the performances of pagure which is why I am thinking of splitting the update of the ticket/request git repos into their own service. However, I think we may want to think about restructuring how our services are running. We'll see whichever comes first :)

ahhh got a lot of insight , good catch on the lost commit one I was wondering why that has been happening ! :)

rebased

:thumbsup:

rebased

Pull-Request has been merged by pingou

Metadata