Page MenuHomePhabricator

Comments are truncated at first non-base-plane character
Closed, ResolvedPublic

Tokens
"Mountain of Wealth" token, awarded by Krenair."Mountain of Wealth" token, awarded by joshuaspence."The World Burns" token, awarded by btrahan."Mountain of Wealth" token, awarded by staticshock."Mountain of Wealth" token, awarded by hach-que."Piece of Eight" token, awarded by chad.
Assigned To
None
Authored By
erling
May 6 2012, 4:53 AM

Description

Posting Unicode characters in Differential comments cause the string to be truncated.

If you do not see a G clef in brackets here, then the same bug applies here: [

Revisions and Commits

rPHU libphutil
D10798
D10623
D10604
D8315
D8313
D8310
rP Phabricator
D10800
D10799
D10797
D10806
D10804
D10786
D10771
D10760
D10758
D10757
D10632
D10627
D10621
D10620
D10619
D10616
D10615
D10611
D10614
D10613
D10612
D10607
D10603
D10602
D10601
D10599
D10598
D10595
D10594
D10593
D10592
D10591
D10590
D10589
D10588
D10587
D10583
D10582
D10581
D10580
D10605
D10577
D10576
D10575
D10528
D10527
D10526
D10525
D10524
D10523
D10521
D10520
D10519
D10518
D10516
D10501
D10500
D10499
D10497
D10496
D10494
D8316
D8314

Related Objects

StatusAssignedTask
Resolvedbtrahan
ResolvedNone
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
fabe added a subscriber: fabe.Oct 2 2014, 4:53 PM

there seems to be some major changes between the mysql versions with the charsets and the syntax.
When running the adjust script against a mysql 5.1 i get:
#1064: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'binary NULL' at line 1

'Collation binary' seems not to be valid for 5.1
when i change just the charset to binary the column type goes to varbinary and the collation is set to 'NULL' (according to a show full columns)

Oh, that's just my fault -- I ran binary migrations but it was a few patches back and they worked, but didn't take them into account after more changes occurred. Let me sort them out locally and you can try again.

beng removed a subscriber: beng.Oct 2 2014, 5:33 PM
epriestley added subscribers: oles, hach-que.

Support Impact This has widespread impact, notably preventing import of repositories with an emoji anywhere in them with no workaround.
High Support Impact Massive data migrations.

Support Impact This has widespread impact, notably preventing import of repositories with an emoji anywhere in them with no workaround.

As a fun note, that now includes the Phabricator repository itself ever since you made rPfda0b086b565aa :)

While I remember:

  • quickstart will currently do the wrong thing for old MySQL.
  • adjust should warn you if you're about to adjust an old MySQL instance, and advise you that updating MySQL first may save you time in the long run.

And:

  • Databases also create with the wrong collation for old MySQL.

Other installs should NOT run bin/storage adjust yet: we have some things that still need cleanup.

What's the status on those things that need cleanup? Looking at the landed diffs, it seems like they have all been taken care of. We don't have old MySQL (5.5.38). Is it (relatively) safe to adjust?

aklapper added a subscriber: aklapper.

Perhaps I'm misunderstanding, but it seems like storage adjust could be made more efficient. Each time you alter a table, MySQL makes a copy of it, alters the copy then replaces the original with the modified one. If we perform several alters, this adds up to several table copies. On large tables this takes considerable time.

I believe that if we ran one alter statement per table it could speed up the process considerably.

Yes, the current implementation doesn't pursue that optimization because it's simpler to omit it and simpler to debug issues when the queries only do one thing each.

I'm getting the following error when trying to apply storage adjustments:

> ./bin/storage adjust
Verifying database schemata...


Database                 Table              Name       Issues     
phabricator_differential differential_draft key_unique Missing Key

Found 1 issues(s) with schemata, detailed above.

You can review issues in more detail from the web interface, in Config > Database Status. To better understand the adjustment workflow, see "Managing Storage Adjustments" in the documentation.

MySQL needs to copy table data to make some adjustments, so these migrations may take some time.


    Fix these schema issues? [y/N] y

Dropping caches, for faster migrations...
Purging remarkup cache...done.
Purging changeset cache...done.
Purging general cache...done.
Fixing schema issues...
Done.                                                                         

Target                                                 Error                                                                                                         
phabricator_differential.differential_draft.key_unique #1062: Duplicate entry 'PHID-DREV-qjg6xlf327vscfghhsy2-PHID-USER-hfenohlvucsbthy2su73-di' for key 'key_unique'

Failed to make some schema adjustments, detailed above.
For help troubleshooting adjustments, see "Managing Storage Adjustments" in the documentation.

I think some user has somehow managed to end up with two drafts for the same revision. The right fix is probably to remove one and then adjust again.

Also want to add that we had several duplicate differential_drafts on our install as well. About 15 objectPHID-authorPHID combinations had duplicates, ranging from 2-10 duplicates each. Cleaning them up was pretty straightforward.

Additionally, one of the adjustments failed for us. Specifically: phabricator_repository.repository_lintmessage.code Wrong Column Type. PHPCS creates codes that are rather silly in length, like "PHPCS.E.Generic.Functions.FunctionCallArgumentSpacing.NoSpaceAfterComma" so the suggest length of 32 would truncate data, so that adjust fails.

Lastly, from IRC, we couldn't save wiki pages, but applied the diff in P1415 and it resolved that problem.

locutus added a subscriber: locutus.Nov 6 2014, 5:56 PM

D10800 marks this as fixed. For more details (or if you run into issues) see T6485 (which will be updated shortly to reflect the current state of the world).

D10804 fixes the wiki issue directly. D10798 (which is the patch in P1415) will also fix it indirectly.

chad added a comment.Nov 7 2014, 10:28 PM

I feel we owe @epriestley many beers and chips for this change. Or a lifetime's worth.

If only beers could be purchased with mountain of wealth tokens... :3

Well, let's see if I destroyed everything before we celebrate too much.

I'm sure there are some lessons here, I'm just not sure what they are. Maybe:

If you're going to implement 3/4ths of utf8, a good name would be "utf8_but_only_somewhat".

There's also some kind of mess with MySQL 6.0 (which is 6 years old but doesn't exist since there was a transition to a "new release model" after Oracle bought MySQL AB?) and renaming utf8 to utf8mb3, and maybe renaming utf8mb4 to utf8. I haven't been able to figure out exactly what the story is here yet, but we should be well-prepared to deal with whatever happens there now that we have the storage adjust infrastructure.

Another thank you note. You guys are great.