fix tablegc flaky test

[shlomi-noach]

Description

Fixes #7984

Hopefully this finally fixes the flaky TestPurge test in CI. The failure almost never reproduces locally, but does rarely. anyway, the solution is based on a bit of extended timeout, and also saves runtime where possible.

What I'm going to do is re-run this particular CI test (tabletmanager_tablegc) a few times, manually, even if successful..

Related Issue(s)

#7984

Checklist

• "Backport me!" label has been added if this change should be backported
• Tests were added or are not required
• Documentation was added or is not required

[github-actions]

Review Checklist

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

General

• Ensure that the Pull Request has the correct release notes label. release notes none should only be used for PRs that are so trivial that they need not be included.
• If a new flag is being introduced, review whether it is really needed. The flag names should be clear and intuitive (as far as possible), and the flag's help should be descriptive.

Bug fixes

• There should be at least one unit or end-to-end test.
• The Pull Request description should either include a link to an issue that describes the bug OR an actual description of the bug and how to reproduce, along with a description of the fix.

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.

[shlomi-noach]

Bummer! After several manual re-runs, I still see flakiness:

2022-05-08T10:39:02.3093006Z === RUN   TestPopulateTable
2022-05-08T10:39:02.3093359Z --- PASS: TestPopulateTable (1.03s)
2022-05-08T10:39:02.3093627Z === RUN   TestHold
2022-05-08T10:39:02.3093918Z     tablegc_test.go:236: 
2022-05-08T10:39:02.3094298Z         	Error Trace:	tablegc_test.go:236
2022-05-08T10:39:02.3094656Z         	Error:      	Should be true
2022-05-08T10:39:02.3094982Z         	Test:       	TestHold
2022-05-08T10:39:02.3095261Z     tablegc_test.go:238: 
2022-05-08T10:39:02.3095655Z         	Error Trace:	tablegc_test.go:238
2022-05-08T10:39:02.3096068Z         	Error:      	Received unexpected error:
2022-05-08T10:39:02.3098131Z         	            	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 '' at line 1 (errno 1064) (sqlstate 42000) during query: drop table if exists
2022-05-08T10:39:02.3098643Z         	Test:       	TestHold
2022-05-08T10:39:02.3098950Z --- FAIL: TestHold (17.03s)
2022-05-08T10:39:02.3099194Z === RUN   TestEvac
2022-05-08T10:39:02.3099481Z --- PASS: TestEvac (23.03s)
2022-05-08T10:39:02.3099720Z === RUN   TestDrop
2022-05-08T10:39:02.3100017Z --- PASS: TestDrop (22.02s)
2022-05-08T10:39:02.3100246Z === RUN   TestPurge
2022-05-08T10:39:02.3100534Z     tablegc_test.go:330: 
2022-05-08T10:39:02.3100930Z         	Error Trace:	tablegc_test.go:330
2022-05-08T10:39:02.3101290Z         	Error:      	Should be true
2022-05-08T10:39:02.3101607Z         	Test:       	TestPurge
2022-05-08T10:39:02.3101929Z --- FAIL: TestPurge (20.03s)
2022-05-08T10:39:02.3102191Z === RUN   TestPurgeView
2022-05-08T10:39:02.3102498Z --- PASS: TestPurgeView (20.03s)
2022-05-08T10:39:02.3102881Z FAIL
2022-05-08T10:39:02.3103200Z FAIL	vitess.io/vitess/go/test/endtoend/tabletmanager/tablegc	122.041s
2022-05-08T10:39:02.3103469Z FAIL

converting to draft while checking it.

[shlomi-noach]

OK this has been successful for multiple successive runs. Maybe solved. Better than before, for sure!
TL;DR: reduced intervals, increased timeouts, allowing multiple states.

[mattlord]

Thank you! ❤️ This is another one that was on my flakes list. 🙂

I had an overall nit about using variables for the time.Sleep() invocations but I will defer to you on whether it's really warranted.

[mattlord]

Nit, but can we make this a variable ~ createTableTimeStampOffsetSecs and then use that everywhere we are doing time.Sleep in relation to that variable? We can then use this throughout:

time.Sleep(createTableTimeStampOffsetSecs * time.Second)
time.Sleep((createTableTimeStampOffsetSecs / 2) * time.Second)
time.Sleep((createTableTimeStampOffsetSecs * 3) * time.Second)
...

This way it's easier to tie all the timing behavior together and adjust it holistically as needed by increasing that one variable.

[shlomi-noach]

All time values now parameterized

[mattlord]

Similar nit here. We could have a variable called purgeReentranceInterval so that those vars are all together and it's clear/easy to adjust the waits.

[shlomi-noach]

done

[shlomi-noach]

aaaand, I've made things worse.

[shlomi-noach]

OK, remade things better