[Django] #23718: TEST_MIRROR setting doesn't work as expected

classic Classic list List threaded Threaded
30 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-------------------------------------+-------------------------------------
     Reporter:  Ilya Baryshev        |                    Owner:  eng.
                                     |  Ilian Iliev
         Type:  Bug                  |                   Status:  assigned
    Component:  Testing framework    |                  Version:  1.7
     Severity:  Normal               |               Resolution:
     Keywords:  replica testing      |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * cc: Carlton Gibson (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:20>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.41208f82329871b94f86b096f005c65b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
In reply to this post by Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-------------------------------------+-------------------------------------
     Reporter:  Ilya Baryshev        |                    Owner:  eng.
                                     |  Ilian Iliev
         Type:  Bug                  |                   Status:  assigned
    Component:  Testing framework    |                  Version:  1.7
     Severity:  Normal               |               Resolution:
     Keywords:  replica testing      |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

 * needs_better_patch:  0 => 1


--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:21>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.c71d8ec12c9306970db9b889e1c80e46%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
In reply to this post by Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-------------------------------------+-------------------------------------
     Reporter:  Ilya Baryshev        |                    Owner:  Simon
                                     |  Charette
         Type:  Bug                  |                   Status:  assigned
    Component:  Testing framework    |                  Version:  1.7
     Severity:  Normal               |               Resolution:
     Keywords:  replica testing      |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

 * 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>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.4186cacbab8e1dc65cc05b1a08769c4c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
In reply to this post by Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-------------------------------------+-------------------------------------
     Reporter:  Ilya Baryshev        |                    Owner:  Simon
                                     |  Charette
         Type:  Bug                  |                   Status:  assigned
    Component:  Testing framework    |                  Version:  1.7
     Severity:  Normal               |               Resolution:
     Keywords:  replica testing      |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

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>
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 [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.4d247397cad7db9cfd6a69bedd911c7e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
In reply to this post by Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-------------------------------------+-------------------------------------
     Reporter:  Ilya Baryshev        |                    Owner:  Simon
                                     |  Charette
         Type:  Bug                  |                   Status:  assigned
    Component:  Testing framework    |                  Version:  1.7
     Severity:  Normal               |               Resolution:
     Keywords:  replica testing      |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

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>
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.08841687e3b7d91047a2ad27d591fcf9%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
In reply to this post by Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-------------------------------------+-------------------------------------
     Reporter:  Ilya Baryshev        |                    Owner:  Simon
                                     |  Charette
         Type:  Bug                  |                   Status:  assigned
    Component:  Testing framework    |                  Version:  1.7
     Severity:  Normal               |               Resolution:
     Keywords:  replica testing      |             Triage Stage:  Accepted
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

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>
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.c2b7294dfb28fdf74dea2cbe7b995a19%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
In reply to this post by Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-----------------------------------+---------------------------------------
     Reporter:  Ilya Baryshev      |                    Owner:  Rag Sagar.V
         Type:  Bug                |                   Status:  assigned
    Component:  Testing framework  |                  Version:  1.7
     Severity:  Normal             |               Resolution:
     Keywords:  replica testing    |             Triage Stage:  Accepted
    Has patch:  1                  |      Needs documentation:  0
  Needs tests:  0                  |  Patch needs improvement:  1
Easy pickings:  0                  |                    UI/UX:  0
-----------------------------------+---------------------------------------
Changes (by Rag Sagar.V):

 * owner:  Simon Charette => Rag Sagar.V


--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:26>
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.45666869f92f99fb5038f126a5796d4c%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
In reply to this post by Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-----------------------------------+-------------------------------------
     Reporter:  Ilya Baryshev      |                    Owner:  Rag Sagar
         Type:  Bug                |                   Status:  assigned
    Component:  Testing framework  |                  Version:  1.7
     Severity:  Normal             |               Resolution:
     Keywords:  replica testing    |             Triage Stage:  Accepted
    Has patch:  1                  |      Needs documentation:  0
  Needs tests:  0                  |  Patch needs improvement:  1
Easy pickings:  0                  |                    UI/UX:  0
-----------------------------------+-------------------------------------

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>
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.b2ec859bb7710b6999e3de115e94fa20%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
In reply to this post by Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-----------------------------------+-------------------------------------
     Reporter:  Ilya Baryshev      |                    Owner:  Rag Sagar
         Type:  Bug                |                   Status:  assigned
    Component:  Testing framework  |                  Version:  1.7
     Severity:  Normal             |               Resolution:
     Keywords:  replica testing    |             Triage Stage:  Accepted
    Has patch:  1                  |      Needs documentation:  0
  Needs tests:  0                  |  Patch needs improvement:  1
Easy pickings:  0                  |                    UI/UX:  0
-----------------------------------+-------------------------------------

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>
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.4c7a25df7cfb6bbce7fdca0f3d5bffff%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #23718: TEST_MIRROR setting doesn't work as expected (and has no tests)

Django
In reply to this post by Django
#23718: TEST_MIRROR setting doesn't work as expected (and has no tests)
-----------------------------------+-------------------------------------
     Reporter:  Ilya Baryshev      |                    Owner:  Rag Sagar
         Type:  Bug                |                   Status:  assigned
    Component:  Testing framework  |                  Version:  1.7
     Severity:  Normal             |               Resolution:
     Keywords:  replica testing    |             Triage Stage:  Accepted
    Has patch:  1                  |      Needs documentation:  0
  Needs tests:  0                  |  Patch needs improvement:  1
Easy pickings:  0                  |                    UI/UX:  0
-----------------------------------+-------------------------------------
Changes (by Patrick Cloke):

 * cc: Patrick Cloke (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/23718#comment:29>
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.5cf2632d367bb5e516656547f5edd52f%40djangoproject.com.
12