Fix flaky TestS3WrongRegionPicked.testS3WrongRegionSelection #14615
Conversation
skrzypo987
force-pushed
the
skrzypo/126-fix-flaky-s3-test
branch
from
1130b13 to
6d09324
Compare
skrzypo987
force-pushed
the
skrzypo/126-fix-flaky-s3-test
branch
from
6d09324 to
35715d8
Compare
Praveen2112
changed the title
| @@ -27,12 +27,14 @@ | |||
| public void testS3WrongRegionSelection() | |||
| throws Exception | |||
| { | |||
| try (HiveMinioDataLake dataLake = new HiveMinioDataLake("test-bucket").start(); | |||
| String bucketName = "test-bucket" + randomTableSuffix(); | |||
There was a problem hiding this comment.
Can we add a comment on why we need this random bucket name ?
There was a problem hiding this comment.
MinioClient has something which is called createdBuckets which is static. Event if we start new Minio container, this will fail to create bucket with same name.
There was a problem hiding this comment.
But in the new minio container, the bucket doesn't exist right ?
There was a problem hiding this comment.
In MinioClient class from package io.trino.testing.minio we have a set of created buckets (which is persistent and independent of minio container and its buckets). Maybe we should clean this set on close?
Edit: If we do this test in loop, same name error will be a thing that will come up as first. I don't know if this is a reason for being flaky.
There was a problem hiding this comment.
This MinioClient class is strictly for testing purposes, so forcing a unique bucket naming seems like a good idea. If I had thought about it in the first place, there would be no bug.
There was a problem hiding this comment.
I would look up on BackgroundHiveSplitLoader if it behaves correctly when stopped and loadSplits is ongoing.
| QueryRunner queryRunner = S3HiveQueryRunner.builder(dataLake) | ||
| .setHiveProperties(ImmutableMap.of("hive.s3.region", "eu-central-1")) // Different than the default one | ||
| .build()) { | ||
| String tableName = "s3_region_test_" + randomTableSuffix(); | ||
| queryRunner.execute("CREATE TABLE default." + tableName + " (a int) WITH (external_location = 's3://test-bucket/" + tableName + "')"); | ||
| queryRunner.execute("CREATE TABLE default." + tableName + " (a int) WITH (external_location = 's3://" + bucketName + "/" + tableName + "')"); |
There was a problem hiding this comment.
nit: Can we useString.format
There was a problem hiding this comment.
I actually prefer concatenation for trivial cases.
skrzypo987
force-pushed
the
skrzypo/126-fix-flaky-s3-test
branch
from
35715d8 to
3cd6340
Compare
Description
The trino-hive testsing suite has been run 5x on github actions and it passes, so it will hopefully work.
Non-technical explanation
Fixes #14568
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: