[Django] #26933: Test for #26804 has a tendency to fail on Oracle databases
Django
-------------------------------------+-------------------------------------
Reporter: jensen-cochran | Owner:
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) | Keywords: update_or_create test
Severity: Normal | tests
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
According to comment on the pull request for #26804
(https://github.com/django/django/pull/6831), the test for the issue
(written by me) has a tendency to fail on Oracle databases. The test
assumes that the database will take no longer than 50ms to lock the
associated row, but it seems that this assumption is incorrect in some
cases.
I have written a patch for this that should mostly fix the issue. Instead
of assuming the database has locked the row, I set a value in the spawned
thread after select_for_update has ran and poll that value in the main
thread.
However, there still has to be some kind of timeout as we cannot wait
forever for the database to lock the row. I have set that timeout at 5
seconds, and if the timeout occurs the test is skipped. Alternatively, I
could fail if the timeout occurs, but this has potential to fail somewhat
randomly as it is doing now. If update_or_create were ever changed such
that the locking behavior was removed, the test would fail so it should
still serve its purpose.
--
Ticket URL: <https://code.djangoproject.com/ticket/26933>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: jensen-cochran | Owner:
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: update_or_create test tests | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
Changes (by jensen-cochran):
* Attachment "26933.diff" added.
Django
-------------------------------------+-------------------------------------
| cochran
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Keywords: update_or_create | Triage Stage:
test tests | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: => jensen-cochran
* needs_better_patch: => 0
* status: new => assigned
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26933#comment:1>
Django
-------------------------------------+-------------------------------------
| cochran
Type: Bug | Status: assigned
(models, ORM) |
Keywords: update_or_create | Triage Stage:
test tests | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* Attachment "26933.diff" added.
Django
-------------------------------------+-------------------------------------
| cochran
Type: Bug | Status: assigned
(models, ORM) |
Keywords: update_or_create | Triage Stage:
test tests | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* Attachment "26933.diff" added.
Django
-------------------------------------+-------------------------------------
| cochran
Type: Bug | Status: assigned
(models, ORM) |
Keywords: update_or_create | Triage Stage:
test tests | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Old description:
> According to comment on the pull request for #26804
> (https://github.com/django/django/pull/6831), the test for the issue
> (written by me) has a tendency to fail on Oracle databases. The test
> assumes that the database will take no longer than 50ms to lock the
> associated row, but it seems that this assumption is incorrect in some
> cases.
>
> I have written a patch for this that should mostly fix the issue.
> Instead of assuming the database has locked the row, I set a value in
> the spawned thread after select_for_update has ran and poll that value in
> the main thread.
>
> However, there still has to be some kind of timeout as we cannot wait
> forever for the database to lock the row. I have set that timeout at 5
> seconds, and if the timeout occurs the test is skipped. Alternatively, I
> could fail if the timeout occurs, but this has potential to fail somewhat
> randomly as it is doing now. If update_or_create were ever changed such
> that the locking behavior was removed, the test would fail so it should
> still serve its purpose.
New description:
According to comment on the pull request for #26804
(https://github.com/django/django/pull/6831), the test for the issue
(written by me) has a tendency to fail on Oracle databases. The test
assumes that the database will take no longer than 50ms to lock the
associated row, but it seems that this assumption is incorrect in some
cases.
I have written a patch for this that should mostly fix the issue. Instead
of assuming the database has locked the row, I set a value in the spawned
thread after select_for_update has ran and poll that value in the main
thread.
However, there still has to be some kind of timeout as we cannot wait
forever for the database to lock the row. I have set that timeout at 0.5
seconds and have changed the thread/callable sleep time to 0.5 seconds as
well to allow for more room for slow databases. There is no reason to
have the timeout be any longer than the thread sleep time, because once
the main thread waits that long the assertion would pass no matter what
anyway. If the timeout expires the test is skipped.
Alternatively, I could fail if the timeout occurs, but this has potential
to fail somewhat randomly as it is doing now.
If update_or_create were ever changed such that the locking behavior was
removed, the test would fail as expected so it should still serve its
purpose.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/26933#comment:2>
Django
-------------------------------------+-------------------------------------
| cochran
Type: Bug | Status: assigned
(models, ORM) |
Keywords: update_or_create | Triage Stage:
test tests | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* Attachment "26933.diff" added.
Django
-------------------------------------+-------------------------------------
| cochran
Type: Bug | Status: assigned
(models, ORM) |
Keywords: update_or_create | Triage Stage: Accepted
test tests |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
For future reference, it would be fine to send a pull request that simply
references the original ticket (since that hasn't been release yet) rather
than opening a new ticket. Can you send a pull request?
--
Ticket URL: <https://code.djangoproject.com/ticket/26933#comment:3>
Django
-------------------------------------+-------------------------------------
| cochran
Type: Bug | Status: assigned
(models, ORM) |
Keywords: update_or_create | Triage Stage: Accepted
test tests |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by jensen-cochran):
Alright thanks, I will make sure to do that in the future. Pull request
is up.
--
Ticket URL: <https://code.djangoproject.com/ticket/26933#comment:4>
Django
-------------------------------------+-------------------------------------
| cochran
Type: Bug | Status: assigned
(models, ORM) |
Keywords: update_or_create | Triage Stage: Accepted
test tests |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
Patch is crashing on my test with Oracle.
--
Ticket URL: <https://code.djangoproject.com/ticket/26933#comment:5>
Django
-------------------------------------+-------------------------------------
| cochran
Component: Database layer | Version: master
(models, ORM) |
Keywords: update_or_create | Triage Stage: Accepted
test tests |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"83be40760a2580db9e5673d111da04f01114aced" 83be4076]:
{{{
#!CommitTicketReference repository=""
revision="83be40760a2580db9e5673d111da04f01114aced"
Fixed #26933 -- Fixed flaky update_or_create() test from refs #26804.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26933#comment:6>