Feature Request: The Transaction Throttler can throttle writes outside of BEGIN/COMMIT

[ejortegau]

Feature Description

Currently, the TxThrottler can trigger throttling only for queries that are inside a BEGIN/COMMITblock if it considers that the query rate is too high to maintain replication lag within acceptable parameters.
This feature request is about making the TxThrottler smarter so that it can throttle without the need for explicit BEGIN/COMMIT commands from the client.

Use Case(s)

Plenty of apps may not open explicit transactions with BEGIN/COMMIT, thus making the current transaction throttler unsuitable for them. With this request, such apps could benefit from the TxThrottler without having to be modified.

[harshit-gangal]

Does this change impact the begin throttler to throttle on query than on begin?

[harshit-gangal]

For Autocommit mode we have a separate execution plan, we can add a throttler check over there.

[ejortegau]

Does this change impact the begin throttler to throttle on query than on begin?

Currently, throttling does not actually take place when the client submits BEGIN; but when it issues a query after having issued a BEGIN. With the proposed change, it would still take place on the query, but without necessarily. having gone through a BEGIN.

[ejortegau]

For Autocommit mode we have a separate execution plan, we can add a throttler check over there.

I will follow your advice in #13040 and move the call to TxThrottler.Throttle() to TxPool.Begin().

[harshit-gangal]

This change is moving the throttling from transaction creation to query execution.

This could possibly lead to a connection to be open and not do anything meaningful to until get rolled back by transaction killer on a transaction timeout.

The reason throttling was done at transaction creation is that the transaction does not open for a long time and if they do get open should be allowed to proceed further and eventually, a commit happens.

With this change, the connections will get blocked to eventually get throttled and rolled back by transaction killer.

I would suggest for this feature to allow read query to pass in the transaction, start the transaction with start transaction read only and in this case, the throttler can be bypassed in the vttablet.

[ejortegau]

I would suggest for this feature to allow read query to pass in the transaction, start the transaction with start transaction read only and in this case, the throttler can be bypassed in the vttablet

If I understand this correctly, you mean to have the throttler decide whether to throttle or not based on whether the current Tx is started as start transaction read only or not instead of checking for the plan type, correct? If so, this leads to client apps needing to be changed, which is something we really want to avoid. Besides, given that in the future we intend to extend the throttler to be able to throttle for other reasons (e.g. connection pool utilization), I do think that we need the throttler to be aware of the plan to decide when it should or should not throttle. For example, if the connection pool is getting close to be exhausted, the throttler should throttle for both reads (SELECT) and writes (INSERT/UPDATE/DELETE/LOAD), whereas if the issue is replication delay, it should only throttle on writes.

[harshit-gangal]

My whole idea is if the connection is taken from the pool, we are not preventing much but just blocking the connection.
e.g.
begin + select - no throttle
update - no throttle
any dml - throttling now

This transaction is held which means locks held and any read query on those rows are anyways not available - which lead to lock wait timeouts.

I think better to throttle on begin+query time. There is no real benefit of throttling later, it can lead to more deadlocks/wait timeouts in MySQL.

Another way of thinking.

begin + select - no throttle
update - throttle

does the application achieve what it wants after reading the record? I believe not. So, still is problematic and does not solve a real case.

if the application executes with select for update the rows are anyways locked to be read by any other connection.

The best way is for the application to tell that they do not want throttling to be effective. I believe this was the same reason PRIORITY was added. So, to use that application anyways needs to be changed.

[ejortegau]

While I understand your concern, I am afraid that expecting the application to be changed in every code path that issues a query is not feasible; I can't speak for other Vitess customers, but for us, there are simply too many - and on top of that, it also involves changing how every dev adds new queries to the codebase when adding or modifying features.

I would propose then doing something different:

1. Leave the call to TxThrottle.Throttle() where it was, so that it is effective in BEGIN + query.
2. Add another call to TxThrottle.Throttle() call in QueryExecutor.execAutocommit() to deal with queries outside of BEGIN/COMMIT blocks.

What do you think?

[harshit-gangal]

Sounds good to me.
The good part of doing this way is that we reach QueryExecutor.execAutocommit() for DML queries only, so the plan type does not need to be checked here.

[ejortegau]

OK, I will proceed as described above

[ejortegau]

It looks like the call to TxThrottler.Throttle() also needs to be added in TabletServer.execAsTransaction(), as running this in the command line:

mysql -c -e  "UPDATE /*vt+ PRIORITY=100 */ sbtest8 SET k=k+1 WHERE id=69294; "

lands there instead of in TabletServer.execAutocommit(). This is a consequence of the plan type being set to UpdateLimit in planbuilder.analyzeUpdate().