Message ID | 20210716171940.3827847-5-dja@axtens.net |
---|---|
State | New |
Headers | show |
Series | RFC: Patchwork 2.2->3.0 migration improvements | expand |
Daniel Axtens <dja@axtens.net> writes: > This saves us doing a whole bunch of rewrites sequentially - instead > we just do one big rewrite. > > Currently breaks postgres and sqlite, which is annoying because they > don't need this to go faster. OTOH it makes mysql a **lot** faster - from > around 220s to ~45s, and that will scale with table size. > > Mostly, however, this is RFC because I am not sure about the random-ish > strings in the middle of the constraint names. I took them from the > `sqlmigrate` output but I don't know if they're random or special. This is a doozy. The random strings are created by names_digest in django. It's the first 8 hex digits of an md5 digest. The inputs to the md5 digest are a bit fiddly to determine. The name of the constraint is created by _fk_constraint_name in django.db.backends.base.schema.BaseDatabaseSchemaEditor. (AFAICT it is not overloaded.) _fk_constraint_name is this: def _fk_constraint_name(self, model, field, suffix): def create_fk_name(*args, **kwargs): return self.quote_name(self._create_index_name(*args, **kwargs)) return ForeignKeyName( model._meta.db_table, [field.column], split_identifier(field.target_field.model._meta.db_table)[1], [field.target_field.column], suffix, create_fk_name, ) Ignoring ForeignKeyName for now, we can infer that create_fk_name will get called and will call _create_index_name. That does this: def _create_index_name(self, table_name, column_names, suffix=""): """ Generate a unique name for an index/unique constraint. The name is divided into 3 parts: the table name, the column names, and a unique digest and suffix. """ _, table_name = split_identifier(table_name) hash_suffix_part = '%s%s' % (names_digest(table_name, *column_names, length=8), suffix) max_length = self.connection.ops.max_name_length() or 200 # If everything fits into max_length, use that name. index_name = '%s_%s_%s' % (table_name, '_'.join(column_names), hash_suffix_part) if len(index_name) <= max_length: return index_name # Shorten a long suffix. if len(hash_suffix_part) > max_length / 3: hash_suffix_part = hash_suffix_part[:max_length // 3] other_length = (max_length - len(hash_suffix_part)) // 2 - 1 index_name = '%s_%s_%s' % ( table_name[:other_length], '_'.join(column_names)[:other_length], hash_suffix_part, ) # Prepend D if needed to prevent the name from starting with an # underscore or a number (not permitted on Oracle). if index_name[0] == "_" or index_name[0].isdigit(): index_name = "D%s" % index_name[:-1] return index_name So we call names_digest() on the table_name and the column_names. So I feel fairly safe to say that it's going to be static and based on the md5 of the table and related columns. I'll try to patch my local django install and see what's getting passed to names_digest to confirm, but we're _probably_ right to just use the names based on the SQL from sqlmigrate. Anyway I am trying to not spend all my nights coding so I will leave it here for now and return to it later. Kind regards, Daniel > > Thoughts? > > Signed-off-by: Daniel Axtens <dja@axtens.net> > --- > .../migrations/0043_merge_patch_submission.py | 195 +++++++++++------- > 1 file changed, 117 insertions(+), 78 deletions(-) > > diff --git a/patchwork/migrations/0043_merge_patch_submission.py b/patchwork/migrations/0043_merge_patch_submission.py > index 1d072ca18230..e17c394f3ffb 100644 > --- a/patchwork/migrations/0043_merge_patch_submission.py > +++ b/patchwork/migrations/0043_merge_patch_submission.py > @@ -4,9 +4,34 @@ import django.db.models.deletion > from django.db.models import Max > > import patchwork.fields > +import datetime > > CHUNK_SIZE = 10000 > > +def report(apps, schema_editor): > + print(datetime.datetime.now()) > + > +def add_fields(apps, schema_editor): > + # mysql - each can be a costly rewrite, bundle. > + # pgsql and others don't let us do multiple which is sad. > + schema_editor.execute( > + """ > + ALTER TABLE `patchwork_submission` > + ADD COLUMN `archived` bool DEFAULT b'0' NOT NULL, > + ADD COLUMN `commit_ref` varchar(255) NULL, > + ADD COLUMN `delegate_id` integer NULL , ADD CONSTRAINT `patchwork_submission_delegate_id_4b8639b8_fk_auth_user_id` FOREIGN KEY (`delegate_id`) REFERENCES `auth_user`(`id`), > + ADD COLUMN `diff` longtext NULL, > + ADD COLUMN `hash` char(40) NULL, > + ADD COLUMN `number` smallint UNSIGNED NULL, > + ADD COLUMN `pull_url` varchar(255) NULL, > + ADD COLUMN `related_id` integer NULL , ADD CONSTRAINT `patchwork_submission_related_id_2e3e66e8_fk_patchwork` FOREIGN KEY (`related_id`) REFERENCES `patchwork_patchrelation`(`id`), > + ADD COLUMN `series_id` integer NULL , ADD CONSTRAINT `patchwork_submission_series_id_61c13d16_fk_patchwork_series_id` FOREIGN KEY (`series_id`) REFERENCES `patchwork_series`(`id`), > + ADD COLUMN `state_id` integer NULL , ADD CONSTRAINT `patchwork_submission_state_id_947655b7_fk_patchwork_state_id` FOREIGN KEY (`state_id`) REFERENCES `patchwork_state`(`id`); > + ALTER TABLE `patchwork_submission` > + ALTER COLUMN `archived` DROP DEFAULT; > + """ > + ) > + > def migrate_data(apps, schema_editor): > Patch = apps.get_model('patchwork', 'patch') > max_id = Patch.objects.all().aggregate(Max('submission_ptr_id'))['submission_ptr_id__max'] > @@ -101,6 +126,7 @@ class Migration(migrations.Migration): > > operations = [ > # move the 'PatchTag' model to point to 'Submission' > + migrations.RunPython(report, None, atomic=False), > > migrations.RemoveField(model_name='patch', name='tags',), > migrations.AddField( > @@ -118,6 +144,7 @@ class Migration(migrations.Migration): > to='patchwork.Submission', > ), > ), > + migrations.RunPython(report, None, atomic=False), > > # do the same for any other field that references 'Patch' > > @@ -166,6 +193,7 @@ class Migration(migrations.Migration): > to='patchwork.Submission', > ), > ), > + migrations.RunPython(report, None, atomic=False), > > # rename all the fields on 'Patch' so we don't have duplicates when we > # add them to 'Submission' > @@ -209,94 +237,104 @@ class Migration(migrations.Migration): > ), > ], > ), > + migrations.RunPython(report, None, atomic=False), > > # add the fields found on 'Patch' to 'Submission' > > - migrations.AddField( > - model_name='submission', > - name='archived', > - field=models.BooleanField(default=False), > - ), > - migrations.AddField( > - model_name='submission', > - name='commit_ref', > - field=models.CharField(blank=True, max_length=255, null=True), > - ), > - migrations.AddField( > - model_name='submission', > - name='delegate', > - field=models.ForeignKey( > - blank=True, > - null=True, > - on_delete=django.db.models.deletion.CASCADE, > - to=settings.AUTH_USER_MODEL, > - ), > - ), > - migrations.AddField( > - model_name='submission', > - name='diff', > - field=models.TextField(blank=True, null=True), > - ), > - migrations.AddField( > - model_name='submission', > - name='hash', > - field=patchwork.fields.HashField( > - blank=True, max_length=40, null=True > - ), > - ), > - migrations.AddField( > - model_name='submission', > - name='number', > - field=models.PositiveSmallIntegerField( > - default=None, > - help_text='The number assigned to this patch in the series', > - null=True, > - ), > - ), > - migrations.AddField( > - model_name='submission', > - name='pull_url', > - field=models.CharField(blank=True, max_length=255, null=True), > - ), > - migrations.AddField( > - model_name='submission', > - name='related', > - field=models.ForeignKey( > - blank=True, > - null=True, > - on_delete=django.db.models.deletion.SET_NULL, > - related_name='patches', > - related_query_name='patch', > - to='patchwork.PatchRelation', > - ), > - ), > - migrations.AddField( > - model_name='submission', > - name='series', > - field=models.ForeignKey( > - blank=True, > - null=True, > - on_delete=django.db.models.deletion.CASCADE, > - related_name='patches', > - related_query_name='patch', > - to='patchwork.Series', > - ), > - ), > - migrations.AddField( > - model_name='submission', > - name='state', > - field=models.ForeignKey( > - null=True, > - on_delete=django.db.models.deletion.CASCADE, > - to='patchwork.State', > - ), > + migrations.SeparateDatabaseAndState( > + state_operations=[ > + migrations.AddField( > + model_name='submission', > + name='archived', > + field=models.BooleanField(default=False), > + ), > + migrations.AddField( > + model_name='submission', > + name='commit_ref', > + field=models.CharField(blank=True, max_length=255, null=True), > + ), > + migrations.AddField( > + model_name='submission', > + name='delegate', > + field=models.ForeignKey( > + blank=True, > + null=True, > + on_delete=django.db.models.deletion.CASCADE, > + to=settings.AUTH_USER_MODEL, > + ), > + ), > + migrations.AddField( > + model_name='submission', > + name='diff', > + field=models.TextField(blank=True, null=True), > + ), > + migrations.AddField( > + model_name='submission', > + name='hash', > + field=patchwork.fields.HashField( > + blank=True, max_length=40, null=True > + ), > + ), > + migrations.AddField( > + model_name='submission', > + name='number', > + field=models.PositiveSmallIntegerField( > + default=None, > + help_text='The number assigned to this patch in the series', > + null=True, > + ), > + ), > + migrations.AddField( > + model_name='submission', > + name='pull_url', > + field=models.CharField(blank=True, max_length=255, null=True), > + ), > + migrations.AddField( > + model_name='submission', > + name='related', > + field=models.ForeignKey( > + blank=True, > + null=True, > + on_delete=django.db.models.deletion.SET_NULL, > + related_name='patches', > + related_query_name='patch', > + to='patchwork.PatchRelation', > + ), > + ), > + migrations.AddField( > + model_name='submission', > + name='series', > + field=models.ForeignKey( > + blank=True, > + null=True, > + on_delete=django.db.models.deletion.CASCADE, > + related_name='patches', > + related_query_name='patch', > + to='patchwork.Series', > + ), > + ), > + migrations.AddField( > + model_name='submission', > + name='state', > + field=models.ForeignKey( > + null=True, > + on_delete=django.db.models.deletion.CASCADE, > + to='patchwork.State', > + ), > + ), > + ], > + database_operations=[ > + migrations.RunPython(add_fields, None, atomic=False), > + ] > ), > > # copy the data from 'Patch' to 'Submission' > + migrations.RunPython(report, None, atomic=False), > > migrations.RunPython(migrate_data, None, atomic=False), > > # configure metadata for the 'Submission' model > + migrations.RunPython(report, None, atomic=False), > > migrations.AlterModelOptions( > name='submission', > @@ -330,6 +368,7 @@ class Migration(migrations.Migration): > ), > > # drop the 'Patch' model and rename 'Submission' to 'Patch' > + migrations.RunPython(report, None, atomic=False), > > migrations.DeleteModel(name='Patch',), > migrations.RenameModel(old_name='Submission', new_name='Patch',), > -- > 2.30.2
diff --git a/patchwork/migrations/0043_merge_patch_submission.py b/patchwork/migrations/0043_merge_patch_submission.py index 1d072ca18230..e17c394f3ffb 100644 --- a/patchwork/migrations/0043_merge_patch_submission.py +++ b/patchwork/migrations/0043_merge_patch_submission.py @@ -4,9 +4,34 @@ import django.db.models.deletion from django.db.models import Max import patchwork.fields +import datetime CHUNK_SIZE = 10000 +def report(apps, schema_editor): + print(datetime.datetime.now()) + +def add_fields(apps, schema_editor): + # mysql - each can be a costly rewrite, bundle. + # pgsql and others don't let us do multiple which is sad. + schema_editor.execute( + """ + ALTER TABLE `patchwork_submission` + ADD COLUMN `archived` bool DEFAULT b'0' NOT NULL, + ADD COLUMN `commit_ref` varchar(255) NULL, + ADD COLUMN `delegate_id` integer NULL , ADD CONSTRAINT `patchwork_submission_delegate_id_4b8639b8_fk_auth_user_id` FOREIGN KEY (`delegate_id`) REFERENCES `auth_user`(`id`), + ADD COLUMN `diff` longtext NULL, + ADD COLUMN `hash` char(40) NULL, + ADD COLUMN `number` smallint UNSIGNED NULL, + ADD COLUMN `pull_url` varchar(255) NULL, + ADD COLUMN `related_id` integer NULL , ADD CONSTRAINT `patchwork_submission_related_id_2e3e66e8_fk_patchwork` FOREIGN KEY (`related_id`) REFERENCES `patchwork_patchrelation`(`id`), + ADD COLUMN `series_id` integer NULL , ADD CONSTRAINT `patchwork_submission_series_id_61c13d16_fk_patchwork_series_id` FOREIGN KEY (`series_id`) REFERENCES `patchwork_series`(`id`), + ADD COLUMN `state_id` integer NULL , ADD CONSTRAINT `patchwork_submission_state_id_947655b7_fk_patchwork_state_id` FOREIGN KEY (`state_id`) REFERENCES `patchwork_state`(`id`); + ALTER TABLE `patchwork_submission` + ALTER COLUMN `archived` DROP DEFAULT; + """ + ) + def migrate_data(apps, schema_editor): Patch = apps.get_model('patchwork', 'patch') max_id = Patch.objects.all().aggregate(Max('submission_ptr_id'))['submission_ptr_id__max'] @@ -101,6 +126,7 @@ class Migration(migrations.Migration): operations = [ # move the 'PatchTag' model to point to 'Submission' + migrations.RunPython(report, None, atomic=False), migrations.RemoveField(model_name='patch', name='tags',), migrations.AddField( @@ -118,6 +144,7 @@ class Migration(migrations.Migration): to='patchwork.Submission', ), ), + migrations.RunPython(report, None, atomic=False), # do the same for any other field that references 'Patch' @@ -166,6 +193,7 @@ class Migration(migrations.Migration): to='patchwork.Submission', ), ), + migrations.RunPython(report, None, atomic=False), # rename all the fields on 'Patch' so we don't have duplicates when we # add them to 'Submission' @@ -209,94 +237,104 @@ class Migration(migrations.Migration): ), ], ), + migrations.RunPython(report, None, atomic=False), # add the fields found on 'Patch' to 'Submission' - migrations.AddField( - model_name='submission', - name='archived', - field=models.BooleanField(default=False), - ), - migrations.AddField( - model_name='submission', - name='commit_ref', - field=models.CharField(blank=True, max_length=255, null=True), - ), - migrations.AddField( - model_name='submission', - name='delegate', - field=models.ForeignKey( - blank=True, - null=True, - on_delete=django.db.models.deletion.CASCADE, - to=settings.AUTH_USER_MODEL, - ), - ), - migrations.AddField( - model_name='submission', - name='diff', - field=models.TextField(blank=True, null=True), - ), - migrations.AddField( - model_name='submission', - name='hash', - field=patchwork.fields.HashField( - blank=True, max_length=40, null=True - ), - ), - migrations.AddField( - model_name='submission', - name='number', - field=models.PositiveSmallIntegerField( - default=None, - help_text='The number assigned to this patch in the series', - null=True, - ), - ), - migrations.AddField( - model_name='submission', - name='pull_url', - field=models.CharField(blank=True, max_length=255, null=True), - ), - migrations.AddField( - model_name='submission', - name='related', - field=models.ForeignKey( - blank=True, - null=True, - on_delete=django.db.models.deletion.SET_NULL, - related_name='patches', - related_query_name='patch', - to='patchwork.PatchRelation', - ), - ), - migrations.AddField( - model_name='submission', - name='series', - field=models.ForeignKey( - blank=True, - null=True, - on_delete=django.db.models.deletion.CASCADE, - related_name='patches', - related_query_name='patch', - to='patchwork.Series', - ), - ), - migrations.AddField( - model_name='submission', - name='state', - field=models.ForeignKey( - null=True, - on_delete=django.db.models.deletion.CASCADE, - to='patchwork.State', - ), + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.AddField( + model_name='submission', + name='archived', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='submission', + name='commit_ref', + field=models.CharField(blank=True, max_length=255, null=True), + ), + migrations.AddField( + model_name='submission', + name='delegate', + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.AddField( + model_name='submission', + name='diff', + field=models.TextField(blank=True, null=True), + ), + migrations.AddField( + model_name='submission', + name='hash', + field=patchwork.fields.HashField( + blank=True, max_length=40, null=True + ), + ), + migrations.AddField( + model_name='submission', + name='number', + field=models.PositiveSmallIntegerField( + default=None, + help_text='The number assigned to this patch in the series', + null=True, + ), + ), + migrations.AddField( + model_name='submission', + name='pull_url', + field=models.CharField(blank=True, max_length=255, null=True), + ), + migrations.AddField( + model_name='submission', + name='related', + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name='patches', + related_query_name='patch', + to='patchwork.PatchRelation', + ), + ), + migrations.AddField( + model_name='submission', + name='series', + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name='patches', + related_query_name='patch', + to='patchwork.Series', + ), + ), + migrations.AddField( + model_name='submission', + name='state', + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + to='patchwork.State', + ), + ), + ], + database_operations=[ + migrations.RunPython(add_fields, None, atomic=False), + ] ), # copy the data from 'Patch' to 'Submission' + migrations.RunPython(report, None, atomic=False), migrations.RunPython(migrate_data, None, atomic=False), # configure metadata for the 'Submission' model + migrations.RunPython(report, None, atomic=False), migrations.AlterModelOptions( name='submission', @@ -330,6 +368,7 @@ class Migration(migrations.Migration): ), # drop the 'Patch' model and rename 'Submission' to 'Patch' + migrations.RunPython(report, None, atomic=False), migrations.DeleteModel(name='Patch',), migrations.RenameModel(old_name='Submission', new_name='Patch',),
This saves us doing a whole bunch of rewrites sequentially - instead we just do one big rewrite. Currently breaks postgres and sqlite, which is annoying because they don't need this to go faster. OTOH it makes mysql a **lot** faster - from around 220s to ~45s, and that will scale with table size. Mostly, however, this is RFC because I am not sure about the random-ish strings in the middle of the constraint names. I took them from the `sqlmigrate` output but I don't know if they're random or special. Thoughts? Signed-off-by: Daniel Axtens <dja@axtens.net> --- .../migrations/0043_merge_patch_submission.py | 195 +++++++++++------- 1 file changed, 117 insertions(+), 78 deletions(-)