#26933: Test for #26804 has a tendency to fail on Oracle databases
-------------------------------------+-------------------------------------
     Reporter:  jensen-cochran       |                    Owner:  jensen-
                                     |  cochran
         Type:  Bug                  |                   Status:  assigned
    Component:  Database layer       |                  Version:  master
  (models, ORM)                      |
     Severity:  Normal               |               Resolution:
     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
-------------------------------------+-------------------------------------
Description changed by jensen-cochran:

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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/072.ab8a5e24a077f4f0689d1b4d36c12f0f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to