mirror of
https://gitlab.com/bramw/baserow.git
synced 2025-04-14 09:08:32 +00:00
Resolve "Memory leak if the table contains a link_row
field"
This commit is contained in:
parent
4900921035
commit
8b56206bf4
6 changed files with 47 additions and 7 deletions
backend
src/baserow/contrib/database
tests/baserow/contrib/database/field
web-frontend/modules/database/components
|
@ -14,7 +14,7 @@ class DatabaseConfig(AppConfig):
|
||||||
will be registered to the apps, but we do not always want that to happen
|
will be registered to the apps, but we do not always want that to happen
|
||||||
because models with the same class name can differ. They are also meant to be
|
because models with the same class name can differ. They are also meant to be
|
||||||
temporary. Removing the model from the cache does not work because if there
|
temporary. Removing the model from the cache does not work because if there
|
||||||
are multiple requests at the same it is not removed from the cache on time
|
are multiple requests at the same, it is not removed from the cache on time
|
||||||
which could result in hard failures. It is also hard to extend the
|
which could result in hard failures. It is also hard to extend the
|
||||||
django.apps.registry.apps so this hack extends the original `register_model`
|
django.apps.registry.apps so this hack extends the original `register_model`
|
||||||
method and it will only call the original `register_model` method if the
|
method and it will only call the original `register_model` method if the
|
||||||
|
@ -24,13 +24,20 @@ class DatabaseConfig(AppConfig):
|
||||||
am happy to hear about it! :)
|
am happy to hear about it! :)
|
||||||
"""
|
"""
|
||||||
|
|
||||||
original = self.apps.register_model
|
original_register_model = self.apps.register_model
|
||||||
|
|
||||||
def register_model(app_label, model):
|
def register_model(app_label, model):
|
||||||
if not hasattr(model, "_generated_table_model") and not hasattr(
|
if not hasattr(model, "_generated_table_model") and not hasattr(
|
||||||
model._meta.auto_created, "_generated_table_model"
|
model._meta.auto_created, "_generated_table_model"
|
||||||
):
|
):
|
||||||
return original(app_label, model)
|
original_register_model(app_label, model)
|
||||||
|
else:
|
||||||
|
# Trigger the pending operations because the original register_model
|
||||||
|
# method also triggers them. Not triggering them can cause a memory
|
||||||
|
# leak because everytime a table model is generated, it will register
|
||||||
|
# new pending operations.
|
||||||
|
self.apps.do_pending_operations(model)
|
||||||
|
self.apps.clear_cache()
|
||||||
|
|
||||||
self.apps.register_model = register_model
|
self.apps.register_model = register_model
|
||||||
|
|
||||||
|
|
|
@ -555,7 +555,6 @@ class LinkRowFieldType(FieldType):
|
||||||
|
|
||||||
# Note that the through model will not be registered with the apps because of
|
# Note that the through model will not be registered with the apps because of
|
||||||
# the `DatabaseConfig.prevent_generated_model_for_registering` hack.
|
# the `DatabaseConfig.prevent_generated_model_for_registering` hack.
|
||||||
|
|
||||||
models.ManyToManyField(
|
models.ManyToManyField(
|
||||||
to=related_model,
|
to=related_model,
|
||||||
related_name=related_name,
|
related_name=related_name,
|
||||||
|
@ -565,8 +564,17 @@ class LinkRowFieldType(FieldType):
|
||||||
db_constraint=False,
|
db_constraint=False,
|
||||||
).contribute_to_class(model, field_name)
|
).contribute_to_class(model, field_name)
|
||||||
|
|
||||||
|
# Trigger the newly created pending operations of all the models related to the
|
||||||
|
# created ManyToManyField. They need to be called manually because normally
|
||||||
|
# they are triggered when a new new model is registered. Not triggering them
|
||||||
|
# can cause a memory leak because everytime a table model is generated, it will
|
||||||
|
# register new pending operations.
|
||||||
|
apps = model._meta.apps
|
||||||
model_field = model._meta.get_field(field_name)
|
model_field = model._meta.get_field(field_name)
|
||||||
model_field.do_related_class(model_field.remote_field.model, None)
|
apps.do_pending_operations(model)
|
||||||
|
apps.do_pending_operations(related_model)
|
||||||
|
apps.do_pending_operations(model_field.remote_field.through)
|
||||||
|
apps.clear_cache()
|
||||||
|
|
||||||
def prepare_values(self, values, user):
|
def prepare_values(self, values, user):
|
||||||
"""
|
"""
|
||||||
|
|
|
@ -4,6 +4,7 @@ from rest_framework.status import HTTP_200_OK, HTTP_204_NO_CONTENT, HTTP_400_BAD
|
||||||
|
|
||||||
from django.shortcuts import reverse
|
from django.shortcuts import reverse
|
||||||
from django.db import connections
|
from django.db import connections
|
||||||
|
from django.apps.registry import apps
|
||||||
|
|
||||||
from baserow.core.handler import CoreHandler
|
from baserow.core.handler import CoreHandler
|
||||||
from baserow.contrib.database.fields.models import Field
|
from baserow.contrib.database.fields.models import Field
|
||||||
|
@ -16,6 +17,29 @@ from baserow.contrib.database.fields.exceptions import (
|
||||||
from baserow.contrib.database.rows.handler import RowHandler
|
from baserow.contrib.database.rows.handler import RowHandler
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.django_db
|
||||||
|
def test_call_apps_registry_pending_operations(data_fixture):
|
||||||
|
user = data_fixture.create_user()
|
||||||
|
database = data_fixture.create_database_application(user=user, name="Placeholder")
|
||||||
|
table = data_fixture.create_database_table(name="Example", database=database)
|
||||||
|
customers_table = data_fixture.create_database_table(
|
||||||
|
name="Customers", database=database
|
||||||
|
)
|
||||||
|
field_handler = FieldHandler()
|
||||||
|
field_handler.create_field(
|
||||||
|
user=user,
|
||||||
|
table=table,
|
||||||
|
type_name="link_row",
|
||||||
|
name="Test",
|
||||||
|
link_row_table=customers_table,
|
||||||
|
)
|
||||||
|
table.get_model()
|
||||||
|
# Make sure that there are no pending operations in the app registry. Because a
|
||||||
|
# Django ManyToManyField registers pending operations every time a table model is
|
||||||
|
# generated, which can causes a memory leak if they are not triggered.
|
||||||
|
assert len(apps._pending_operations) == 0
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.django_db
|
@pytest.mark.django_db
|
||||||
def test_link_row_field_type(data_fixture):
|
def test_link_row_field_type(data_fixture):
|
||||||
user = data_fixture.create_user()
|
user = data_fixture.create_user()
|
||||||
|
|
|
@ -2,6 +2,7 @@
|
||||||
|
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
|
* Fixed memory leak in the `link_row` field.
|
||||||
* Switch to using a celery based email backend by default.
|
* Switch to using a celery based email backend by default.
|
||||||
* Added `--add-columns` flag to the `fill_table` management command. It creates all the
|
* Added `--add-columns` flag to the `fill_table` management command. It creates all the
|
||||||
field types before filling the table with random data.
|
field types before filling the table with random data.
|
||||||
|
|
|
@ -143,8 +143,8 @@ export default {
|
||||||
required: true,
|
required: true,
|
||||||
},
|
},
|
||||||
view: {
|
view: {
|
||||||
type: Object,
|
|
||||||
validator: (prop) => typeof prop === 'object' || prop === undefined,
|
validator: (prop) => typeof prop === 'object' || prop === undefined,
|
||||||
|
required: true,
|
||||||
},
|
},
|
||||||
tableLoading: {
|
tableLoading: {
|
||||||
type: Boolean,
|
type: Boolean,
|
||||||
|
|
|
@ -55,7 +55,7 @@
|
||||||
</li>
|
</li>
|
||||||
</ul>
|
</ul>
|
||||||
<UserFilesModal
|
<UserFilesModal
|
||||||
v-if="Array.isArray(value) && !this.readOnly"
|
v-if="Array.isArray(value) && !readOnly"
|
||||||
ref="uploadModal"
|
ref="uploadModal"
|
||||||
@uploaded="addFiles(value, $event)"
|
@uploaded="addFiles(value, $event)"
|
||||||
@hidden="hideModal"
|
@hidden="hideModal"
|
||||||
|
|
Loading…
Add table
Reference in a new issue