Skip to content

Bug 1193137: Remove commit_manually decorator from app version update task.#653

Merged
magopian merged 1 commit into
mozilla:masterfrom
kmaglione:update-appsupport
Aug 12, 2015
Merged

Bug 1193137: Remove commit_manually decorator from app version update task.#653
magopian merged 1 commit into
mozilla:masterfrom
kmaglione:update-appsupport

Conversation

@kmaglione
Copy link
Copy Markdown
Contributor

As far as I can tell, what's happening on production is that, because we have a transaction.atomic block inside of a transaction.commit_manually block in the update_addon_appsupport cron job, we always end that block with a pending commit.

When that happens, in our version of django, anyway, the connection also seems to sometimes keep the in_atomic_block flag, which leaves that worker broken for any task that runs next.

I've confirmed that this change fixes at least the first part of the problem. That cron job no longer ends with a transaction error. I haven't been able to reproduce the TransactionManagementError: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block. error in a local web instance yet, but I think this should fix that as well.

@kmaglione
Copy link
Copy Markdown
Contributor Author

I tried to add a test for this. Unfortunately, the transaction management wrappers of the test harness mean that we never actually commit any transactions, and the exit transaction code that throws this error is never called. I can't think of a good way around this.

@magopian
Copy link
Copy Markdown
Contributor

r+

@magopian
Copy link
Copy Markdown
Contributor

I'm going to merge so the QA team has enough time to validate that before the micro-push

magopian added a commit that referenced this pull request Aug 12, 2015
Bug 1193137: Remove `commit_manually` decorator from app version update task.
@magopian magopian merged commit b106467 into mozilla:master Aug 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants