Online DDL: ensure message is valid utf8 in updateMigrationMessage()

[shlomi-noach]

Description

Fixes #11913

This PR ensures to clean up non-utf8 characters from the error message reported in _vt.schema_migrations. If writing the message still fails for whatever reason (the text is too long?), we opt for an alternative, generic error message.

Related Issue(s)

#11913
#6926

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added or are not required
• Documentation was added or is not required

Deployment Notes

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.

If a new flag is being introduced:

• Is it really necessary to add this flag?
• Flag names should be clear and intuitive (as far as possible)
• Help text should be descriptive.
• Flag names should use dashes (-) as word separators rather than underscores (_).

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow should be required, the maintainer team should be notified.

Bug fixes

• There should be at least one unit or end-to-end test.
• The Pull Request description should include a link to an issue that describes the bug.

Non-trivial changes

• There should be some code comments as to why things are implemented the way they are.

New/Existing features

• Should be documented, either by modifying the existing documentation or creating new documentation.
• New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• vtctl command output order should be stable and awk-able.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from VTop, if used there.

[dbussink]

If writing the message still fails for whatever reason (the text is too long?), we opt for an alternative, generic error message.

If we already know length to be a potential issue, should we already always truncate the string then anyway? And do that change here as well?

[dbussink]

Do we want to use � as the replacement here since that's the canonical UTF-8 replacement character (and also what a console / log today would already print).

See also https://www.fileformat.info/info/unicode/char/fffd/index.htm

[shlomi-noach]

@dbussink sounds good. I'm down for maintenance, would you like to push that change?

[shlomi-noach]

done

[shlomi-noach]

truncating length to 2048

[dbussink]

@systay Looks like your recent fixes for the planner are triggered in the upgrade tests (from https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592):

 === RUN   TestSubqueriesExists
[654](https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592#step:18:655)
    subquery_test.go:69: 
[655](https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592#step:18:656)
        	Error Trace:	/home/runner/work/vitess/vitess/go/test/endtoend/vtgate/queries/subquery/cmp.go:194
[656](https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592#step:18:657)
        	            				/home/runner/work/vitess/vitess/go/test/endtoend/vtgate/queries/subquery/cmp.go:66
[657](https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592#step:18:658)
        	            				/home/runner/work/vitess/vitess/go/test/endtoend/vtgate/queries/subquery/subquery_test.go:69
[658](https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592#step:18:659)
        	Error:      	Received unexpected error:
[659](https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592#step:18:660)
        	            	Gen4 failed while v3 did not: unsupported: unable to split predicates to derived table: :__sq_has_values1 (errno 1815) (sqlstate HY000) during query: select * from (select 1) as tmp where exists(select 1 from t1 where id1 = 1)
[660](https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592#step:18:661)
        	Test:       	TestSubqueriesExists
[661](https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592#step:18:662)
        	Messages:   	[Vitess Error] for query: select * from (select 1) as tmp where exists(select 1 from t1 where id1 = 1)

Not sure if that would be fixed if this PR is updated with latest main or what would need to happen to fix that?

The issue is not related to the changes here in this PR though.

[dbussink]

@shlomi-noach Realizing that we should change the order here I think. Since slicing this can then break the last character if the slicing happens on a word boundary and it still creates invalidly encoded data.

[dbussink]

Since message is a TEXT, 16383 would be a safe maxlen. But we still then need to flip the order, so first truncate and then convert to valid UTF-8. It could lead to a � at the end if then we break the last potentially valid unicode char, but I think that's a totally ok trade off if the message is that long already anyway.

[shlomi-noach]

done

[rohit-nayak-ps]

lgtm

[systay]

@systay Looks like your recent fixes for the planner are triggered in the upgrade tests (from https://github.com/vitessio/vitess/actions/runs/3648727440/jobs/6203228592):

#11923 should have fixed this