Skip to content

Cleanup Glue databases left behind by tests #10901

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.

Sign up for GitHub

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

Closed
findepi wants to merge 1 commit into from
Closed

Cleanup Glue databases left behind by tests #10901

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 44 additions & 1 deletion ...n/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@

import com.amazonaws.services.glue.AWSGlueAsync;
import com.amazonaws.services.glue.AWSGlueAsyncClientBuilder;
import com.amazonaws.services.glue.model.Database;
import com.amazonaws.services.glue.model.DeleteDatabaseRequest;
import com.amazonaws.services.glue.model.EntityNotFoundException;
import com.amazonaws.services.glue.model.GetDatabasesRequest;
import com.amazonaws.services.glue.model.GetDatabasesResult;
import com.amazonaws.services.glue.model.TableInput;
import com.amazonaws.services.glue.model.UpdateTableRequest;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import io.airlift.concurrent.BoundedExecutor;
import io.airlift.log.Logger;
import io.airlift.slice.Slice;
import io.trino.plugin.hive.AbstractTestHiveLocal;
import io.trino.plugin.hive.HiveBasicStatistics;
Expand Down Expand Up @@ -81,6 +87,7 @@
import static io.trino.plugin.hive.HiveTestUtils.HDFS_ENVIRONMENT;
import static io.trino.plugin.hive.acid.AcidTransaction.NO_ACID_TRANSACTION;
import static io.trino.plugin.hive.metastore.HiveColumnStatistics.createIntegerColumnStatistics;
import static io.trino.plugin.hive.metastore.glue.AwsSdkUtil.getPaginatedResults;
import static io.trino.plugin.hive.metastore.glue.PartitionFilterBuilder.DECIMAL_TYPE;
import static io.trino.plugin.hive.metastore.glue.PartitionFilterBuilder.decimalOf;
import static io.trino.spi.connector.RetryMode.NO_RETRIES;
Expand All @@ -92,8 +99,10 @@
import static io.trino.spi.type.VarcharType.VARCHAR;
import static io.trino.testing.TestingConnectorSession.SESSION;
import static java.lang.String.format;
import static java.lang.System.currentTimeMillis;
import static java.util.Locale.ENGLISH;
import static java.util.UUID.randomUUID;
import static java.util.concurrent.TimeUnit.DAYS;
import static org.apache.hadoop.hive.common.FileUtils.makePartName;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
Expand All @@ -109,9 +118,12 @@
public class TestHiveGlueMetastore
extends AbstractTestHiveLocal
{
private static final Logger log = Logger.get(TestHiveGlueMetastore.class);

private static final HiveIdentity HIVE_IDENTITY = new HiveIdentity(SESSION);
private static final String PARTITION_KEY = "part_key_1";
private static final String PARTITION_KEY2 = "part_key_2";
private static final String TEST_DATABASE_NAME_PREFIX = "test_glue";

private static final List<ColumnMetadata> CREATE_TABLE_COLUMNS = ImmutableList.<ColumnMetadata>builder()
.add(new ColumnMetadata("id", BigintType.BIGINT))
Expand Down Expand Up @@ -163,7 +175,7 @@ public class TestHiveGlueMetastore

public TestHiveGlueMetastore()
{
super("test_glue" + randomUUID().toString().toLowerCase(ENGLISH).replace("-", ""));
super(TEST_DATABASE_NAME_PREFIX + randomUUID().toString().toLowerCase(ENGLISH).replace("-", ""));
}

protected AWSGlueAsync getGlueClient()
Expand Down Expand Up @@ -207,6 +219,37 @@ protected HiveMetastore createMetastore(File tempDir)
.setHideDeltaLakeTables(true)).get());
}

@Test
public void cleanupOrphanedDatabases()
Copy link
Copy Markdown
Member

@hashhar hashhar Feb 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Runs in parallel. Any databases being listed in this test class can then become flaky.
Would be nice to fix those kind of tests (but I didn't spend time to find them).

Alternatively we make this just a method and call in @BeforeClass? (still flaky if other CI runs are running - but lesser chances)

Copy link
Copy Markdown
Member Author

@findepi findepi Feb 2, 2022

Choose a reason for hiding this comment

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

There is time threshold, so it doesn't matter whether this is @Test or @BeforeClass.

It still runs in parallel with other runs (test processes), that's why i tried to code this defensively.

{
long creationTimeMillisThreshold = currentTimeMillis() - DAYS.toMillis(1);
List<String> orphanedDatabases = getPaginatedResults(
glueClient::getDatabases,
new GetDatabasesRequest(),
GetDatabasesRequest::setNextToken,
GetDatabasesResult::getNextToken)
.map(GetDatabasesResult::getDatabaseList)
.flatMap(List::stream)
.filter(database -> database.getName().startsWith(TEST_DATABASE_NAME_PREFIX) &&
database.getCreateTime().getTime() <= creationTimeMillisThreshold)
.map(Database::getName)
.collect(toImmutableList());

log.info("Found %s %s* databases that look orphaned, removing", orphanedDatabases.size(), TEST_DATABASE_NAME_PREFIX);
orphanedDatabases.forEach(database -> {
try {
glueClient.deleteDatabase(new DeleteDatabaseRequest()
.withName(database));
}
catch (EntityNotFoundException e) {
log.info("Database [%s] not found, could be removed by other cleanup process", database);
}
catch (RuntimeException e) {
log.warn(e, "Failed to remove database [%s]", database);
}
});
}

@Override
public void testRenameTable()
{
Expand Down