Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#16490 closed Bug (fixed)

For select_for_update feature: DatabaseError not raised as expected when NOWAIT not supported; mult thread test also failing (on MySQL)

Reported by: jsdalton Owned by: nobody
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: jim.dalton@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

The following test fails on MySQL InnoDB. This appears to relate to the work recently done on #2705.

$ ./runtests.py --settings=testproject.settings select_for_update.SelectForUpdateTests
Creating test database for alias 'default'...
Creating test database for alias 'other'...
F.s...F
======================================================================
FAIL: test_block (modeltests.select_for_update.tests.SelectForUpdateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jsdalton/webs/testproject/src/django/django/test/testcases.py", line 615, in skip_wrapper
    return test_func(*args, **kwargs)
  File "/Users/jsdalton/webs/testproject/src/django/django/test/testcases.py", line 615, in skip_wrapper
    return test_func(*args, **kwargs)
  File "/Users/jsdalton/webs/testproject/src/django/tests/modeltests/select_for_update/tests.py", line 209, in test_block
    self.assertEqual('Fred', p.name)
AssertionError: 'Fred' != u'Reinhardt'

======================================================================
FAIL: test_unsupported_nowait_raises_error (modeltests.select_for_update.tests.SelectForUpdateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jsdalton/webs/testproject/src/django/django/test/testcases.py", line 615, in skip_wrapper
    return test_func(*args, **kwargs)
  File "/Users/jsdalton/webs/testproject/src/django/django/test/testcases.py", line 615, in skip_wrapper
    return test_func(*args, **kwargs)
  File "/Users/jsdalton/webs/testproject/src/django/tests/modeltests/select_for_update/tests.py", line 138, in test_unsupported_nowait_raises_error
    Person.objects.all().select_for_update(nowait=True)
AssertionError: DatabaseError not raised

----------------------------------------------------------------------
Ran 7 tests in 10.342s

FAILED (failures=2, skipped=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

The DatabaseError is not being raised because -- well, I wish I knew. The code block is being executed but the error is being caught, i.e. I can print to stdout but any exception is not being raised. The thread related error is also something I'm unclear on.

As part of the work I am doing right now on #11665, these two issues are the *only* remaining failures in the MySQL test suite. Seriously, the only remaining failures. Given that this feature seems to be quite new, I'm hopeful that it's fresh on everyone's minds and easy enough to address, perhaps by the folks who worked so hard on #2705. If I can be of any assistance, let me know.

Attachments (1)

fix_test_block_mysql_issue.diff (753 bytes) - added by jsdalton 4 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 4 years ago by jsdalton

  • Cc jim.dalton@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 4 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 4 years ago by ramiro

I see the first error (modeltests.select_for_update.tests.SelectForUpdateTests.test_block) when running tests with a pristine checkout of trunk against MySQL 5.1.58 so this doesn't seem to be introduced by your patch for #11665. Graham King tells us he doesn't see it with Maybe tehre is some change of behavior in MySQL betwen 5.1.54 and 5.1.58? What version are you using?

But I don't see the second one (modeltests.select_for_update.tests.SelectForUpdateTests.test_unsupported_nowait_raises_error), this is strange because the SelectForUpdateTests tests inherits from TransactionTestCase and in theory shoudn't be affected by your changes to TestCase handling of transactions in #11665.

I tried with both Python 2.6 and 2.7, MySQL storage backend is InnoDB.

Version 0, edited 4 years ago by ramiro (next)

comment:4 Changed 4 years ago by jsdalton

Well, hm. I upgraded to Lion last night which may have changed my MySQL version, so I no longer can replicate the conditions under which the original report occurred. I do know that I was running it under an up-to-date master branch.

Regardless, I can no longer replicate the NO WAIT error, but I do get the other one test_block() still. I'm happy about that because I was absolutely confounded by that other error, so now I can chalk it up to a fluke :).

So, yes, one test failure now, and this is running Python 2.6.6, MySQL 5.1.40, InnoDB.

comment:5 Changed 4 years ago by kmtracey

  • Resolution set to duplicate
  • Status changed from new to closed

The first failure is due to repeatable read, see #13906. The 2nd failure I can't recreate either. I'm going to close this one as a dupe of #13906; we should figure out what to do about the repeatable read issue and that will either fix the first failure or involve "fixing" this test to avoid the problem. If anyone else sees the test_unsupported_nowait_raises_error failure we can reopen this one to address that here.

comment:6 Changed 4 years ago by jsdalton

  • Easy pickings set
  • Has patch set
  • Resolution duplicate deleted
  • Status changed from closed to reopened

After doing some investigation on #13906, I've come to the conclusion that this issue does not need to wait for a decision about REPEATABLE READ to be made in order to be addressed. A simple transaction.commit() in the test_block() method before the final select will ensure that MySQL performs a fresh read. The test runs cleanly now on all backends with no side effects from the commit (since no data is actually being inserted or updated on this connection). Note that transaction.rollback() would also have the same effect, but in my opinion that makes the test a bit more confusing to the casual observer, since it makes it look like we're rolling back the work we just did.

Patch attached, this should be an easy one.

Changed 4 years ago by jsdalton

comment:7 Changed 4 years ago by ramiro

  • Component changed from Database layer (models, ORM) to Testing framework

comment:8 Changed 4 years ago by adamnelson

  • Triage Stage changed from Accepted to Ready for checkin

patch applies and test does not fail for me on Lion with MySQL 5.5.14

comment:9 Changed 4 years ago by russellm

In [16781]:

Refs #16490 - Add a commit to ensure that a fresh read is provided; this is only a problem for databases in REPEATABLE READ mode, which is MySQL InnoDB's default. Thanks to Jim Dalton for the report and patch.

comment:10 Changed 4 years ago by russellm

I've worked out the cause of the other problem -- it's the old "Python 2.6.1 eats exceptions" problem. The DatabaseError raised when NOWAIT isn't available is eaten by the iteration process in execute_sql. This only affects Python 2.6.1; our approach on other instances of this problem has been to skip the test, rather than try and work around a bug that exists in one version of Python.

A fix will be landing shortly.

comment:11 Changed 4 years ago by russellm

For the record, the Python bug is http://bugs.python.org/issue1242657

comment:12 Changed 4 years ago by russellm

  • Resolution set to fixed
  • Status changed from reopened to closed

In [16784]:

Fixed #16490 -- Skipped a test failure that only occurs under Python 2.6.1 (it's the old iteration-eats-exceptions problem).

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