Opened 8 years ago

Closed 8 years ago

#26933 closed Bug (fixed)

Test for #26804 has a tendency to fail on Oracle databases

Reported by: Jensen Cochran Owned by: Jensen Cochran
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: update_or_create test tests
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description (last modified by Jensen Cochran)

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.

Attachments (1)

26933.diff (2.1 KB ) - added by Jensen Cochran 8 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 by Jensen Cochran, 8 years ago

Owner: set to Jensen Cochran
Status: newassigned

comment:2 by Jensen Cochran, 8 years ago

Description: modified (diff)

by Jensen Cochran, 8 years ago

Attachment: 26933.diff added

comment:3 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted

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?

comment:4 by Jensen Cochran, 8 years ago

Alright thanks, I will make sure to do that in the future. Pull request is up.

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:5 by Tim Graham, 8 years ago

Patch needs improvement: set

Patch is crashing on my test with Oracle.

comment:6 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 83be4076:

Fixed #26933 -- Fixed flaky update_or_create() test from refs #26804.

Note: See TracTickets for help on using tickets.
Back to Top