Use migrations to adjust the database schema to be ready for use with… by DeepDiver1975 · Pull Request #546 · owncloud/activity

DeepDiver1975 · 2016-11-22T11:07:12Z

… mysql mb4

mention-bot · 2016-11-22T11:07:13Z

@DeepDiver1975, thanks for your PR! By analyzing the history of the files in this pull request, we identified @janklostermann, @nickvergessen and @PVince81 to be potential reviewers.

butonic · 2016-11-25T08:19:06Z

+			$this->addSql("ALTER TABLE $tableName ADD (\"tmpsubjectparams\" CLOB, \"tmpmessageparams\" CLOB)");
+			$this->addSql("UPDATE $tableName SET \"tmpsubjectparams\"=\"subjectparams\", \"tmpmessageparams\"=\"messageparams\"");
+			$this->addSql("COMMIT");
+			$this->addSql("ALTER TABLE $tableName DROP COLUMN \"subjectparams\"");


DROP takes time. Instead of DROP rename it to a different name, eg "oldsubjectparams" and "oldmessageparams", then rename the temp colums. This keeps the gap to a minimum ... the old* colums can then be deleted after the migration ...

basically we have to decide if we want a fast migration (which requires more disk space and a subsequent cleanup) or a migration that keeps a clean state ...

I vote for fast.

In the case of an overblown activity table I guess it could be several GBs for a duplicate table, not sure if this is acceptable.

no, you need the space anyway because you have to duplicate the columns anyway ...

butonic · 2016-11-25T08:23:29Z

+		$tableName = "{$prefix}activity";
+		if ($this->connection->getDatabasePlatform() instanceof OraclePlatform) {
+			$tableName = $this->connection->quoteIdentifier($tableName);
+			$this->addSql("ALTER TABLE $tableName ADD (\"tmpsubjectparams\" CLOB, \"tmpmessageparams\" CLOB)");


if you are not using $this->connection->quoteIdentifier('tmpsubjectparams'); you might as well use ALTER TABLE \"$tableName\" ...

do not make these colums CLOB, then we might as well say activities is unsupported on oracle. In https://github.com/owncloud/activity/blob/master/lib/Data.php#L207 we execute a SELECT * which causes the automatic conversion of OCI-Lob objects into strings. In the case of the appconfig table that easily adds a millisecond per row, for ~120 rows that makes 120ms ON EVERY REQUEST. Here we have the activity table with thousands of rows and two CLOBS. The default limit is 30 ... or 50 ... I would expect 60-100ms for these queries.

in https://github.com/owncloud/activity/blob/master/lib/Data.php#L303 we only need affecteduser and timestamp, get rid of the *: fixed with select only necessary colums #547

alse LIKE does not work on CLOB columns: https://github.com/owncloud/activity/blob/master/appinfo/update.php#L38 in this case we might get away with it if the cleanup happens before this up script.

ok, CLOB might work ... performance will suck big time compared to the open source databases ... but might be acceptable.

I guess we can remove these update steps - 1.2.2 is oc 8.1 afaik

DeepDiver1975 · 2016-12-15T00:09:50Z

ALL GREEN!

@PhilippSchaffrath @PVince81 @butonic please review and test! THX

phisch · 2016-12-15T10:15:44Z

+class Version20161122085340 extends AbstractMigration {
+
+	/**
+     * @param Schema $schema


Indentation swaps somewhat from tabs to spaces here and in the rest of this file.

This is because the migration generator uses spaces instead of tabs...

… mysql mb4

PVince81 · 2017-02-02T10:38:38Z

MariaDB [owncloud]> select * from oc_activity where file like '%☻%';
+-------------+------------+----------+--------------+-------+--------------+-------+--------------+---------------------------------------+---------+---------------+---------------+-----------------------------------------------------------+-------------+-----------+
| activity_id | timestamp  | priority | type         | user  | affecteduser | app   | subject      | subjectparams                         | message | messageparams | file          | link                                                      | object_type | object_id |
+-------------+------------+----------+--------------+-------+--------------+-------+--------------+---------------------------------------+---------+---------------+---------------+-----------------------------------------------------------+-------------+-----------+
|           8 | 1486031825 |       30 | file_created | user1 | user1        | files | created_self | [{"26":"\/test\/\u263b.txt"}]         |         | []            | /test/☻.txt   | http://localhost/owncloud/index.php/apps/files/?dir=/test | files       |        26 |
|           9 | 1486031825 |       30 | file_created | user1 | admin        | files | created_by   | [{"26":"\/test\/\u263b.txt"},"user1"] |         | []            | /test/☻.txt   | http://localhost/owncloud/index.php/apps/files/?dir=/test | files       |        26 |
|          10 | 1486031828 |       30 | file_changed | user1 | user1        | files | changed_self | [{"26":"\/test\/\u263b.txt"}]         |         | []            | /test/☻.txt   | http://localhost/owncloud/index.php/apps/files/?dir=/test | files       |        26 |
|          11 | 1486031828 |       30 | file_changed | user1 | admin        | files | changed_by   | [{"26":"\/test\/\u263b.txt"},"user1"] |         | []            | /test/☻.txt   | http://localhost/owncloud/index.php/apps/files/?dir=/test | files       |        26 |
+-------------+------------+----------+--------------+-------+--------------+-------+--------------+---------------------------------------+---------+---------------+---------------+-----------------------------------------------------------+-------------+-----------+
4 rows in set (0.00 sec)

👍

DeepDiver1975 added the 3 - To review label Nov 22, 2016

DeepDiver1975 added this to the 9.2 milestone Nov 22, 2016

DeepDiver1975 mentioned this pull request Nov 22, 2016

doctrine migrations owncloud/core#14851

Merged

7 tasks

DeepDiver1975 force-pushed the introduce-migrations-to-allow-mb4 branch 21 times, most recently from 0feef87 to 6740643 Compare November 24, 2016 23:24

butonic reviewed Nov 25, 2016

View reviewed changes

butonic mentioned this pull request Nov 25, 2016

select only necessary colums #547

Merged

DeepDiver1975 force-pushed the introduce-migrations-to-allow-mb4 branch 2 times, most recently from bc96af3 to 7e4584c Compare December 14, 2016 22:01

DeepDiver1975 force-pushed the introduce-migrations-to-allow-mb4 branch 2 times, most recently from 70c4fa3 to dc2fa45 Compare December 14, 2016 23:27

DeepDiver1975 requested a review from phisch December 15, 2016 00:10

phisch reviewed Dec 15, 2016

View reviewed changes

DeepDiver1975 force-pushed the introduce-migrations-to-allow-mb4 branch 2 times, most recently from 24e2621 to 3d13eba Compare December 22, 2016 11:49

LukasReschke mentioned this pull request Dec 22, 2016

Poc doctrine migrations nextcloud/server#2827

Closed

DeepDiver1975 added 3 commits January 31, 2017 15:19

Use migrations to adjust the database schema to be ready for use with…

6983ef7

… mysql mb4

Test on oracle

bc17580

Use own phpunit in the correct version

aa08cdc

DeepDiver1975 force-pushed the introduce-migrations-to-allow-mb4 branch from 727775a to 3ce6b06 Compare January 31, 2017 14:20

Test on mysqlmb4

a972dc4

DeepDiver1975 force-pushed the introduce-migrations-to-allow-mb4 branch from 755bf16 to a972dc4 Compare February 7, 2017 12:00

PVince81 merged commit f559b0c into master Feb 7, 2017

PVince81 deleted the introduce-migrations-to-allow-mb4 branch February 7, 2017 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use migrations to adjust the database schema to be ready for use with…#546

Use migrations to adjust the database schema to be ready for use with…#546
PVince81 merged 4 commits into
masterfrom
introduce-migrations-to-allow-mb4

DeepDiver1975 commented Nov 22, 2016

Uh oh!

mention-bot commented Nov 22, 2016

Uh oh!

butonic Nov 25, 2016

Uh oh!

PVince81 Nov 25, 2016

Uh oh!

butonic Nov 25, 2016

Uh oh!

butonic Nov 25, 2016

Uh oh!

butonic Nov 25, 2016 •

edited

Loading

Uh oh!

butonic Nov 25, 2016

Uh oh!

DeepDiver1975 Nov 25, 2016

Uh oh!

DeepDiver1975 commented Dec 15, 2016

Uh oh!

phisch Dec 15, 2016

Uh oh!

PVince81 Dec 15, 2016

Uh oh!

PVince81 commented Feb 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

DeepDiver1975 commented Nov 22, 2016

Uh oh!

mention-bot commented Nov 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

butonic Nov 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 commented Dec 15, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PVince81 commented Feb 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

butonic Nov 25, 2016 •

edited

Loading