{{{
def test_fixture(self):
MyModel.objects.using('default').create(name=1)
MyModel.objects.using('slave').create(name=2)
MyModel.objects.using('slave').create(name=3)
self.assertEqual(list(map(repr,
MyModel.objects.using('default').all())),
list(map(repr,
MyModel.objects.using('slave').all())))
}}}
Both lists should be equal, because replica queries should be hitting
default instead.
It appears not to be the case for Django>=1.4 up to latest 1.7.1 (but
actually passes against 1.3.7)
{{{
AssertionError: Lists differ: ['<MyModel: 1>', '<MyModel: 2>... !=
['<MyModel: 2>', '<MyModel: 3>...
First differing element 0:
<MyModel: 1>
<MyModel: 2>
First list contains 1 additional elements.
First extra element 2:
<MyModel: 3>
- ['<MyModel: 1>', '<MyModel: 2>', '<MyModel: 3>']
? ^ ----------------
+ ['<MyModel: 2>', '<MyModel: 3>']
? ^
}}}
Here is a project I used to test: https://github.com/coagulant/test_mirror
--
Ticket URL: <https://code.djangoproject.com/ticket/23718>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
It would be quite helpful if you could convert your test to one for
Django's test suite and then bisect to determine the commit in Django that
introduced the regression.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:1>
Comment (by coagulant):
I've run git bisect, and first commit with regression happened to be:
[https://github.com/django/django/commit/905e33f84a1a10e4f0183d879c52076ef876fc3b
905e33f84a1a10e4f0183d879c52076ef876fc3b], which is related to #16885
Here is a log of my bisect:
https://gist.github.com/coagulant/d04b3e729b851784a93a
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:2>
* stage: Unreviewed => Accepted
Comment:
I haven't verified the issue, but I noticed there are no tests for
`TEST['MIRROR']` (renamed on master) so we should at least add some. Also
the test in the "regression commit" seems to be gone -- not sure if its
removal was intentional or not.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:3>
Comment (by Atala):
Ran into the same issue, I think both the old (TEST_MIRROR) and new (TEST:
{MIRROR:) settings are not working. I had the issue running raw sql with
the connection cursor.
I am new in this part of the codebase & in databases connections, so I am
not really sure what's going on. In this gist:
https://gist.github.com/Atala/d5420ac426c79b45d4db , it's clear that the
two connections returned are not the same object (they should have same ID
no?) but are pointing to the same DB name. However a SELECT statement
will work only on the 'default' db, a SELECT against 'prestashop' will
return nothing (but the tables are correctly created).
I would be glad to help if you had some hints,
Aloïs
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:4>
Comment (by Atala):
Replying to [comment:4 Atala]:
> I deleted my comment, I was running the tests with django_nose test
runner - just noticed it today. Still having issues with mirroring during
tests, I will try to investigate more later on.
>
> Aloïs
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:5>
Comment (by shaib):
The behavior in the bug description is sort-of expected, because of
transactions (`default` and `slave` are the same database -- in the test
project, they are defined this way even regardless of `TEST_MIRROR` -- but
accessed through different connections, and so, separate transactions).
I'm saying sort-of expected because it seems the test is running in a
transaction on `default` but not on `slave`, which is a little surprising
-- but not the problem claimed.
I don't think we want to force test-cases to run each test in transactions
on all databases -- I'm not sure that even makes sense; but we should
probably document that tests using more than one database should be
`TransactionTestCase`s.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:6>
Comment (by harrykao):
Is it possible to have the mirrors share a connection? Using
`TransactionTestCase` has the additional disadvantage of preventing
multiprocessed test runs, which works fine when each test runs in a
transaction.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:7>
Comment (by yellottyellott):
I just ran into this as well. Based on the documentation, I expected to
set
{{{
DATABASES = {
'default': {
...
},
'replica': {
...
'TEST_MIRROR': 'default',
# 'TEST': {'MIRROR': 'default'}, # I tried this too.
}
}
}}}
and have my tests read from replica and write to default, which should be
the same database and act as if I had just one database defined as
`default`.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:8>
Comment (by aaugustin):
I just rediscovered that test mirrors require `TransactionTestCase`.
I'm wondering if it would be possible to share the database connection
instead of having two connections with the same parameters.
If that doesn't work, we can document this limitation instead.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:9>
Comment (by asser):
Just ran into this, but with raw queries instead. Using the connection
directly does not work even with {{{TransactionTestCase}}}.
So for now I'm using this hacky workaround:
{{{
#!python
from django.db import connections
class ReplicationTestCase(TestCase):
"""
Redirect raw queries to the replica, ie.
from django.db import connections
cursor = connections['replica'].cursor()
cursor.execute(...) # Is run directly on master in tests
"""
def setUp(self):
super(ReplicationTestCase, self).setUp()
connections['replica']._orig_cursor =
connections['replica'].cursor
connections['replica'].cursor = connections['master'].cursor
def tearDown(self):
connections['replica'].cursor =
connections['replica']._orig_cursor
super(ReplicationTestCase, self).tearDown()
}}}
As an added benefit it works for model managers too even though I'm not
using {{{TransactionTestCase}}} indicating that the connection might be
able to be shared in a similar way?
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:10>
Comment (by Kyle Agronick):
Is this ever going to be fixed? Setting the mirror should just copy the
cursor so no matter where the slave DB is called it redirects to the
master.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:11>
Comment (by Tim Graham):
It will be fixed when someone submits a patch and another person reviews
it.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:12>
Comment (by eng. Ilian Iliev):
I have created a small patch, the PR is here
https://github.com/django/django/pull/9603
The solution I went for is that when the `MIRROR` is specified then the
mirror uses the same connection as the mirrored one.
If the tests look a bit redundant please let me know.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:13>
* status: new => assigned
* owner: nobody => eng. Ilian Iliev
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:14>
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:15>
* needs_better_patch: 0 => 1
Comment:
Comments on PR
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:16>
Comment (by eng. Ilian Iliev):
Patch has been improved - https://github.com/django/django/pull/9603
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:17>
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:18>
* cc: Carlton Gibson (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:20>
Comment (by Carlton Gibson):
Question from PR:
The PR currently uses mocks in the tests, but it's **much** simpler (for
the current test and presumably into the future) and much clearer to just
declare an extra `replica` alias with the `TEST["MIRROR"]` setting.
This second approach would require adjustments to test settings for
Jenkins, the Django-Box VM and all devs' settings running against other
DBs.
So the question is can we make that change?
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:19>
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:21>
* owner: eng. Ilian Iliev => Simon Charette
Comment:
Planning to work on this since affecting our use of testing using
`TEST_MIRROR` and `TestCase`.
I think the right approach here is to have mirror connections swapped to
the connection object they mirror '''but only when using TestCase''' to
allow proper transaction testing to take place. That's something the
previous patch wasn't doing appropriately.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:22>
Comment (by pachewise):
Hi Team,
We also noticed an issue here which happens when you have a default +
read_replica config, and you have a router that only selects a
read_replica when considering particular models. If you have those
conditions, _and_ a data migration that uses both Job and User, you will
have issues because the "default" connection config will be pointing to
your `test_myschema`, whereas the `read_replica` config will still be
pointing to `myschema`.
e.g.,
{{{
models.py:
Job:
user_id := FK(User)
DBRouter:
db_for_write := 'read_replica' if model == Job else 'default'
settings:
DATABASES = {
'default': { ... },
'read_replica': {
# <configs>...
'TEST': {'MIRROR': 'default'}
}
}
0001_migration.py:
# ...
for job in Job.objects.all(): # using apps.get_model()
job.user.id # will error out with __fake__.DoesNotExist
}}}
In the case above, if your "live" read_replica schema has any job data, it
will try to run that last line, but there won't be any Users in the
`test_schema` to match to.
Not sure if ''we're'' doing something wrong, but just letting you know
that there are cases where a custom DBrouter could route to a config that
''should'' be a test mirror, but isn't.
Using python 3.6, Django 1.11.20, MySQL 5.7
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:23>
Comment (by Rag Sagar.V):
@Simon Charette If you haven't started working on this, I would like to
give this a try.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:24>
Comment (by Simon Charette):
@Rag Sagar.V, sure feel free to assign the ticket to yourself.
I'd start by having a look at [https://github.com/django/django/pull/9603
the previous PR] and make sure to understand the rationale behind why it
got rejected.
For what it's worth I gave this ticket a 2-3 hours try by trying to point
`connections['mirror']` at `connections['mirrored']` Python objects for
the duration of `TestCase` and I got some failures related to transaction
handling. I think
[https://github.com/django/django/pull/9603#issuecomment-388347770 Tim did
a good job at nailing the issues] with this approach. In short transaction
handing is alias based so if something goes wrong with
`connections['mirrored']` then it's also surfaced in
`connections['mirror']` which differs from what would happen in a real
life scenario.
The further I got from getting things working were by setting the
isolation level on the miror/replica to `READ UNCOMMITED` for the duration
of the test to allow it to see changes within the testcase transaction.
This isolation level is not supported on all backends though (e.g.
PostgreSQL) and was quite unreliable on MySQL from my minimal testing.
That one might be hard one if you're not familiar with how `TestCase`
transactions wrapping takes place but you'll definitely learn a lot by
giving this ticket a shot and seeing how the suite fails. Happy to review
your changes if you get stuck on Github.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:25>
* owner: Simon Charette => Rag Sagar.V
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:26>
Comment (by Rag Sagar):
@Simon Charette Since `READ_UNCOMMITED` is not supported in postgres,
Should I explore further in that direction? The solution that comes up on
my mind is pointing mirror connections to mirrored for the duration of the
`TestCase`. But that has the issues you pointed out. So I am bit confused
which way I should go.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:27>
Comment (by Simon Charette):
Rag Sagar, I'm afraid none of the above solution will work appropriately.
Here's the problem:
If you configure mirrors to create a new connection to mirrored connection
settings then changes performed within a transaction using the mirrored
connection won't be visible in the mirror connection per `READ COMMITED`
transaction isolation definition. That's the case right now.
Using `READ UNCOMMITTED` on the mirror connection seems to be a no-go from
limited support and the fact MySQL seems inconsistent and reusing the same
connection has confuses transaction management.
The only solutions I see going forward are:
1. Document this caveats of `TEST_MIRROR` and `TestCase` transaction
wrapping interactions.
2. Invest more time trying to get the `connections` repointing during
`TestCase.setUpClass` working to get the suite passing and document the
new caveats.
In both cases we could at least do 1. or warn about it to make it clear
this is not currently supported.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:28>
* cc: Patrick Cloke (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:29>
Comment (by Mariusz Felisiak):
#33153 was a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:30>
* owner: Rag Sagar => Christian Bundy
* needs_better_patch: 1 => 0
* component: Testing framework => Documentation
Comment:
[https://github.com/django/django/pull/14811 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:31>
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:32>
Comment (by Mariusz Felisiak):
#34193 was a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:33>
* owner: Christian Bundy => Sarah Boyce
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/16458 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:34>
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:35>
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"d550e3cccd77ceaea0d77fd2df472454a238a62e" d550e3c]:
{{{
#!CommitTicketReference repository=""
revision="d550e3cccd77ceaea0d77fd2df472454a238a62e"
[4.1.x] Fixed #23718 -- Doc'd that test mirrors require
TransactionTestCase.
Co-authored-by: Christian Bundy <m...@christianbundy.com>
Backport of 0fbdb9784da915fce5dcc1fe82bac9b4785749e5 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:36>
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"0fbdb9784da915fce5dcc1fe82bac9b4785749e5" 0fbdb97]:
{{{
#!CommitTicketReference repository=""
revision="0fbdb9784da915fce5dcc1fe82bac9b4785749e5"
Fixed #23718 -- Doc'd that test mirrors require TransactionTestCase.
Co-authored-by: Christian Bundy <m...@christianbundy.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:37>