Atomic Transactions handling with Online DDL

[harshit-gangal]

Description

This PR handles the case for atomic transactions and online ddl to work together during cutover so that atomic guarantees are not broken.
Online DDL will wait for any existing prepared atomic transactions to be completed and prevent any new transactions from being prepared.

Related Issue(s)

• RFC: Atomic Distributed Transactions #16245

Checklist

• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on CI?
• Documentation was added or is not required

[GrahamCampbell]

Does vtgate attempt to buffer these queries?

[GuptaManan100]

This is a prepare call for a distributed transaction that is trying to be committed. If this call fails, then the transaction will be rollbacked.

[codecov]

Codecov Report

Attention: Patch coverage is 73.18841% with 37 lines in your changes missing coverage. Please review.

Project coverage is 68.92%. Comparing base (e6843dc) to head (1f13e01).
Report is 17 commits behind head on main.

Files	Patch %	Lines
go/vt/vttablet/onlineddl/executor.go	6.25%	15 Missing ⚠️
go/vt/vttablet/tabletserver/tx_prep_pool.go	56.66%	13 Missing ⚠️
go/vt/vttablet/tabletserver/tx/api.go	78.94%	4 Missing ⚠️
go/vt/vttablet/tabletserver/dt_executor.go	85.71%	3 Missing ⚠️
go/vt/vttablet/tabletserver/tabletserver.go	81.81%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16585      +/-   ##
==========================================
- Coverage   68.99%   68.92%   -0.07%     
==========================================
  Files        1562     1562              
  Lines      200754   200850      +96     
==========================================
- Hits       138508   138435      -73     
- Misses      62246    62415     +169     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[shlomi-noach]

Let's discuss the race condition again: this function is called when query buffering is already in place, which technically prevents a new prepared transaction. And given we also intentionnal sleep(100*time.Milisecond) in between, I don't see how realistically this might happen, but:

• Say a prepared transaction gets as far as this line:
  https://github.com/vitessio/vitess/pull/16585/files#diff-a1efa75583f31e230076c9385a551deb725040bbbf85085833be7c14a7a18055R102
• Say Online DDL cut-over begins and starts buffering.
• The prepared transaction above proceeds a bit further.
• When Online DDL invokes IsEmpty(), will it see the prepared transaction above? If yes, fine. If not, then it might kill it.

[harshit-gangal]

Now we give even prepared pool to get empty in 100ms. This will handle the case a transaction gets added to prepared pool after query rules are added.
Within this 100ms, it will recheck the rules and fail the prepare step, allowing online ddl cutover to proceed.

[harshit-gangal]

the other check if if if gets past the first query rule check and not added to prepared pool list, then online DDL will kill the connection. This is also handled as we check if the MySQL connection is closed before proceeding.

[shlomi-noach]

the other check if if if gets past the first query rule check and not added to prepared pool list

The race condition is what happens if it gets past the first check, then Online DDL doesn't see it in the prepared pool list and proceeds towards the killing function, then the query gets into the prepared pool list, then Online DDL actually finds it and kills it.

[harshit-gangal]

The list is just a first check point.
The prepare happens latter on when we update the database with the redo logs. Till then any failure is accepted