Change GRPC message limit SQL error type

[setassociative]

Summary

There are a collection of errors bundled under the generic GRPC ResourceExhausted error. Several of them are not necessarily related to resource exhausted but to limits that are being exceeded in a way that backing off and waiting for reduced load will not resolve.

This change introduces a processing step in the GRPC -> mysql error conversion that rewrites those errors from the current 1203 - ERTooManyUserConnections to a (somewhat) more appropriate 1153 - ERNetPacketTooLarge. Two other errors are also recategorized in the vtgate executor to be treated as the 1153 instead of 1203.

The three errors being changed:

• When a GRPC message coming back to the vtgate exceeds the configured size limit (configured via grpc_max_message_size)
• If a write is attempting to make a change with a payload that is larger than the configured allowable size
• if a scatter query is attempting to collect process too many rows in memory within the vtgate to construct a response

Before this change each of these would the SQL error ERTooManyUserConnections when using the MySQL interface. This is a fairly unfortunate conflation of that error and doesn't make sense to be consumed as "too many users" when the actual error is various flavors of "too much data."

Further in a situation where you want to use error codes to apply back pressure to callers having these two classes of error combined means that it's not a great signal.

We discussed briefly whether we should also change the GRPC error so that ResourceExhausted isn't overloaded but it seems that those errors need to match GRPC error set (ref). Additionally it would be a much larger change because ResourceExhausted is used for some session management. If we decide later to revisit this decision that will be fine as the GRPC and SQL errors are independent (also, even if it does feel like what we'd want to do I would prefer it to be a separate change from this from a pragmatic standpoint).

Misc

1️⃣ This change exceeds our initial plan which covered only the GRPC message limit case and I'm open for pushback on the other two but I do think that all these cases are better suited as 1153 than 1203.

2️⃣ The error message checks are extremely factored and I'm happy to pull them out of vterrors for another home or deal with that differently if folks have opinions. I didn't know what was the most reasonable location since they needed to be shared between sql_errors and the vtgate

Testing

• Added unit tests for the method that processes the resource exhausted errors
• Built and deployed a modified vtgate and observed expected (new) error code when running with an artificially low grpc message limit (3k). Note ERROR 1153 in the following:

mysql> select id, data from test_table limit 100;
ERROR 1153 (HY000): vtgate: http://vtgate-dev/: target: keyspace.-.master, used tablet: <tablet_uid> (<tablet-host>): vttablet: rpc error: code = ResourceExhausted desc = grpc: received message larger than max (12579 vs. 3072)

• Observed row-count limit related errors returning 1153 mysql error:

mysql> select id, data from test_table limit 10000;
ERROR 1153 (HY000): vtgate: http://vtgate-dev/: in-memory row count exceeded allowed limit of 100

• Explicitly: did not test the payload-too-large case and relying on unit tests 🤠

Reference

We talked about this in the Vitess slack and decided on ERNetPacketTooLarge as the new error code there.

Release Note Required

A MySQL error code 1153 is now being returned in the following cases:

• When a GRPC message coming back to the vtgate exceeds the configured size limit
• If a write is attempting to make a change with a payload that is larger than the configured allowable size
• if a scatter query is attempting to process too many rows in memory within the vtgate constructing the result set

Previously these would be returned as MySQL 1203 if received over the MySQL connector or Code_RESOURCE_EXHAUSTED over GRPC.

[setassociative]

@harshit-gangal I applied suggested error code changes; I believe the unit test that is failing is unrelated since, afaict, it's erroring out with relation to failing to connect to a mysql instance. Possible due to bad auth?

[deepthi]

@setassociative this could break applications that depend on the exact error code? Can you summarize the net effect in a Release Note Required section so that we can put it into the release notes?

[harshit-gangal]

is this still required after changing to through mysql error directly?

[setassociative]

Yes -- if we directly map all ResourceExhausted into ERNetPacketTooLarge then we end up in the same position where we are grouping errors that don't necessarily make sense together.

Specifically when we overflow a connection pool (vttablet: rpc error: code = ResourceExhausted desc = pool ConnPool waiter count exceeded) and the like we want to distinguish that as something that can be solved via backoff (ResourceExhausted) instead of queries that are inherently less retryable because they are likely to generate "bad" data (ErNetPacketTooLarge).

[harshit-gangal]

all of them should say ERNetPacketTooLarge. you should change the pattern matcher. I do not see a reason why the first test case should not say ERNetPacketTooLarge

[setassociative]

✅ Sure thing