Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix BaseFailureRecoveryTest temp table checking flakiness #16277 #16499

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

mwd410
Copy link
Contributor

@mwd410 mwd410 commented Mar 10, 2023

Description

This fixes the issue described in #16277

Because information_schema is not strictly consistent with recently dropped tables among all connectors, but querying recently dropped tables is consistent, this no longer relies on SHOW TABLES LIKE returning no results, but instead, actually attempts to query the recently dropped temporary table, and expects there to be a "does not exist" exception.

Additional context and related issues

Release notes

(X) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@mwd410 mwd410 force-pushed the mdeady-fix-fte-temp-tables-16277 branch from 1b64fd4 to da6c9c1 Compare March 10, 2023 18:30
Copy link
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat. Thanks

@@ -13,6 +13,7 @@
*/
package io.trino.testing;

import com.google.common.base.Joiner;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please drop issue from commit message

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

temporaryTableNamePrefix(entry.getKey()),
Joiner.on(",").join(entry.getValue()),
Joiner.on("],\n[").join(assertionErrorMessages.get(entry.getKey())).replace("\n", "\n\t\t\t")))
.collect(joining("\n")))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example message from this:

There should be no remaining tmp_trino tables that are queryable. They are:
	For queryId [20230310_183735_00015_ii5en] (prefix [tmp_trino_31f5781f_]) remaining tables: [tmp_trino_31f5781f_14b3eb70]
		With errors: [
			Expecting code to raise a throwable.]
	For queryId [20230310_183738_00020_ii5en] (prefix [tmp_trino_5ce0903c_]) remaining tables: [tmp_trino_5ce0903c_2f4304bc]
		With errors: [
			Expecting code to raise a throwable.]

@mwd410 mwd410 force-pushed the mdeady-fix-fte-temp-tables-16277 branch from da6c9c1 to 58b1208 Compare March 10, 2023 18:40
Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, much better than what I came up with.

LGTM. Can you add a stress test with just the MySQL FTE tests having 30 runs. I was able to get it to fail around 1 in 10 times against MySQL.

@mwd410 mwd410 force-pushed the mdeady-fix-fte-temp-tables-16277 branch from 58b1208 to e1c915a Compare March 10, 2023 20:37
@mwd410
Copy link
Contributor Author

mwd410 commented Mar 10, 2023

@hashhar - just pushed the additional mysql only testing 30x CTAS.

@hashhar
Copy link
Member

hashhar commented Mar 11, 2023

Nice, looks good to go. Please don't forget to revert the MySQL changes (I just wanted them to make sure the change is effective before merging).

@mwd410 mwd410 force-pushed the mdeady-fix-fte-temp-tables-16277 branch from e1c915a to 88c2901 Compare March 13, 2023 17:47
@mwd410
Copy link
Contributor Author

mwd410 commented Mar 13, 2023

@hashhar done

@hashhar hashhar merged commit 8f7fe5a into trinodb:master Mar 14, 2023
@mwd410 mwd410 deleted the mdeady-fix-fte-temp-tables-16277 branch March 14, 2023 13:58
@github-actions github-actions bot added this to the 411 milestone Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants