Investigate git extraction errors

[EnTeQuAk]

There will most certainly be filed separate issues but I'll open a meta issue to document my findings while looking at this.

I'm more specifically talking about the following sentry exceptions:

Potentially transaction related

• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5525326/?query=is:unresolved%20git - repository hasn't been properly created before accessing it, should have been fixed by git extraction happens outside of "create new version" transaction #6570, will monitor after Thursday
• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5741699/ - might actually have been fixed by git extraction happens outside of "create new version" transaction #6570, will monitor after Thursday

Pygit/libgit related

• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5526890/ - old reference value does not match - this seems to relate to some new additions in libgit 0.28 which I haven't seen that are adding more strict validation

Potentially EFS/NFS related

• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5525342/ - lock file is stale
• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5525343/
• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5541369/
• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5525655/ - seems to be a race-condition somehow for when the directory will be created and when it'll be accessed
• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5713574/ - looks like another race condition
• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5713487/ - potentially race condition
• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5741906/
• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5714025/
• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5740085/

Other

• https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5741907/ - looks like a deleted user, need to check how that can actually happening. Will be needed to be fixed for the mass extraction

[diox]

For the extraction not finding the Version, we figured out what was going on:

Code path	Release channel	from_upload behaviour
devhub	unlisted	in-request (_submit_upload)
devhub	listed	in-request (_submit_upload)
API	unlisted	task (handle_upload(submit=True) creates a task that calls create_version_for_upload())
API	listed	task (handle_upload(submit=True) creates a task that calls create_version_for_upload())

Version created from the API are created in a task, by create_version_for_upload, which calls sign_file, which calls extract_version_to_git.delay(). The problem is, create_version_for_upload is decorated with a @transaction.atomic, so the transaction might not have been committed when we reach the call to extract_version_to_git.delay().

The solution is to make that call on_commit, and make sure sign_file() is always called inside a transaction.

[EnTeQuAk]

So, I'd like to do the extreme here… there is a setting in our code called NFS_LAG_DELAY and I'd like to re-use that for the extraction. My current plan would be:

• use flufl.lock to create a lock specific to a version that would be extracted
• potentially sleep for a short amount of time (or NFS_LAG_DELAY) to ensure the lock is actually set - although flufl.lock ensures that as much as possible
• create git repository (if not existing)
  • wait NFS_LAG_DELAY again to ensure all .git files exist properly
• create branch (if not existing)
  • wait NFS_LAG_DELAY again to ensure all .git files get updated properly
• create temporary worktree
  • wait NFS_LAG_DELAY again to ensure all .git files get updated properly
• extract all files from XPI/ZIP
  • wait NFS_LAG_DELAY again to ensure all files got written properly
• git add -A and commit

This is a very simplistic representation, ideally this means that we're going to lock and wait after the following methods: AddonGitRepository.{git_repository (if not existing), find_or_create_branch, _commit_through_worktree}

That ideally fixes all or most of the problems by cleanly writing all the data to EFS.

Another alternative to fix this would be to introduce some kind of proxy that accepts all the reads/writes and queues them properly, but that'll make things even more complex than they already are.

[tapaswenipathak]

Hello folks: Can I help or is the ticket open?

[EnTeQuAk]

Hello folks: Can I help or is the ticket open?

No, this issue isn't really suitable for contributors as it requires quite a few experimentation - locally as well as on our servers.

Please take a look at other issues that have the contrib: welcome label.

[EnTeQuAk]

Without actually implementing a extraction-level fix yet, the failure rate has been lowered significantly since November 21st because we enabled delayed unlisted signing.

That only means we're on the right track here with delaying the extraction and improving locking.

Specifically…

https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5525655/

https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5525343/

https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5525326/

https://sentry.prod.mozaws.net/operations/olympia-prod/issues/5525342/

Have simply been "fixed" now. These 4 errors have been the origin of ~708k entries in sentry in the last ~8 months

[willdurand]

I'm going to close this issue and I'll file new ones, specifically one for each error. We fixed a bunch of errors already, but new ones appeared it seems.