Revert "Fixed #16865 -- Made get_or_create use read database for initial get query."
Thanks to Jeremy Dunck for pointing out the problem with this change. If in a
single transaction, the master deletes a record and then get_or_creates a
similar record, under the new behavior the get_or_create would find the record
in the slave db and fail to re-create it, leaving the record nonexistent, which
violates the contract of get_or_create that the record should always exist
afterwards. We need to do everything against the master here in order to ensure
correctness.
This reverts commit 901af86550
.
This commit is contained in:
parent
901af86550
commit
4e9a74b81d
|
@ -455,9 +455,9 @@ class QuerySet(object):
|
||||||
if f.attname in lookup:
|
if f.attname in lookup:
|
||||||
lookup[f.name] = lookup.pop(f.attname)
|
lookup[f.name] = lookup.pop(f.attname)
|
||||||
try:
|
try:
|
||||||
|
self._for_write = True
|
||||||
return self.get(**lookup), False
|
return self.get(**lookup), False
|
||||||
except self.model.DoesNotExist:
|
except self.model.DoesNotExist:
|
||||||
self._for_write = True
|
|
||||||
try:
|
try:
|
||||||
params = dict([(k, v) for k, v in kwargs.items() if '__' not in k])
|
params = dict([(k, v) for k, v in kwargs.items() if '__' not in k])
|
||||||
params.update(defaults)
|
params.update(defaults)
|
||||||
|
|
|
@ -338,15 +338,6 @@ Miscellaneous
|
||||||
needs. The new default value is `0666` (octal) and the current umask value
|
needs. The new default value is `0666` (octal) and the current umask value
|
||||||
is first masked out.
|
is first masked out.
|
||||||
|
|
||||||
* In a multi-database situation, ``get_or_create()`` will now use a read
|
|
||||||
database for the initial ``get`` attempt (previously, it used only the write
|
|
||||||
database for all queries). This change reduces load on the write (master)
|
|
||||||
database, in exchange for slightly more frequent false-negatives on the
|
|
||||||
initial ``get`` due to replication lag. In those cases the subsequent insert
|
|
||||||
will still go to the master and fail, after which the existing object will be
|
|
||||||
fetched from the master.
|
|
||||||
|
|
||||||
|
|
||||||
Features deprecated in 1.5
|
Features deprecated in 1.5
|
||||||
==========================
|
==========================
|
||||||
|
|
||||||
|
|
|
@ -64,27 +64,3 @@ class GetOrCreateTests(TestCase):
|
||||||
formatted_traceback = traceback.format_exc()
|
formatted_traceback = traceback.format_exc()
|
||||||
self.assertIn('obj.save', formatted_traceback)
|
self.assertIn('obj.save', formatted_traceback)
|
||||||
|
|
||||||
|
|
||||||
def test_initial_get_on_read_db(self):
|
|
||||||
"""
|
|
||||||
get_or_create should only set _for_write when it's actually doing a
|
|
||||||
create action. This makes sure that the initial .get() will be able to
|
|
||||||
use a slave database. Specially when some form of database pinning is
|
|
||||||
in place this will help to not put all the SELECT queries on the
|
|
||||||
master. Refs #16865.
|
|
||||||
|
|
||||||
"""
|
|
||||||
qs = Person.objects.get_query_set()
|
|
||||||
p, created = qs.get_or_create(
|
|
||||||
first_name="Stuart", last_name="Sutcliffe", defaults={
|
|
||||||
"birthday": date(1940, 6, 23),
|
|
||||||
}
|
|
||||||
)
|
|
||||||
self.assertTrue(created)
|
|
||||||
self.assertTrue(qs._for_write)
|
|
||||||
|
|
||||||
qs = Person.objects.get_query_set()
|
|
||||||
p, created = qs.get_or_create(
|
|
||||||
first_name="Stuart", last_name="Sutcliffe")
|
|
||||||
self.assertFalse(created)
|
|
||||||
self.assertFalse(qs._for_write)
|
|
||||||
|
|
Loading…
Reference in New Issue