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 flaky BigQuery tests using snapshot tables #18357

Merged
merged 2 commits into from
Jul 24, 2023

Conversation

hashhar
Copy link
Member

@hashhar hashhar commented Jul 20, 2023

Description

BigQuery has limits on how many snapshots or clones can exist for a single table. It seems BigQuery is miscounting though because we occasionally see errors like below on CI even though all snapshots get dropped by their respective tests:

Caused by: com.google.api.client.googleapis.json.GoogleJsonResponseException: 400 Bad Request
POST https://www.googleapis.com/bigquery/v2/projects/sep-bq-cicd/queries
{
  "code": 400,
  "errors": [
    {
      "domain": "global",
      "message": "Cannot have more than 1000 active clones and snapshots of a table. Please use a deep copy, or remove some active clones or snapshots.",
      "reason": "invalid"
    }
  ],
  "message": "Cannot have more than 1000 active clones and snapshots of a table. Please use a deep copy, or remove some active clones or snapshots.",
  "status": "INVALID_ARGUMENT"
}

A workaround is to use a different table as the source of the snapshot everytime which is what this change does.

Fixes #18266

Release notes

(x) This is not user-visible or docs only and no release notes are required.

@hashhar hashhar added the bigquery BigQuery connector label Jul 20, 2023
@hashhar hashhar requested review from wendigo and ebyhr July 20, 2023 10:49
@cla-bot cla-bot bot added the cla-signed label Jul 20, 2023
{VIEW, "CREATE VIEW %s %s", "SELECT * FROM tpch.region", "DROP VIEW %s"},
{MATERIALIZED_VIEW, "CREATE MATERIALIZED VIEW %s %s", "AS SELECT * FROM tpch.region", "DROP MATERIALIZED VIEW %s"},
{EXTERNAL, "CREATE EXTERNAL TABLE %s %s", "OPTIONS (format = 'CSV', uris = ['gs://" + gcpStorageBucket + "/tpch/tiny/region.csv'])", "DROP EXTERNAL TABLE %s"},
{SNAPSHOT, "CREATE SNAPSHOT TABLE %s %s", "CLONE %s", "DROP SNAPSHOT TABLE %s"},
Copy link
Member Author

Choose a reason for hiding this comment

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

couldn't think of a cleaner alternative than parameterizing the "table definition" as well. Suggestions welcome.

Copy link
Member

@ebyhr ebyhr Jul 20, 2023

Choose a reason for hiding this comment

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

How about split into several test methods? DataProvider is helpful for simple cases, but the new code is a little hard to read for me.

    @Test
    public void testEmptyProjectionTable()
    {
        testEmptyProjection(
                tableName -> onBigQuery("CREATE TABLE " + tableName + " AS SELECT * FROM tpch.region"),
                tableName -> onBigQuery("DROP TABLE " + tableName));
    }

... 

    @Test
    public void testEmptyProjectionSnapshot()
    {
        String regionCopy = TEST_SCHEMA + ".region_" + randomNameSuffix();
        testEmptyProjection(
                tableName -> {
                    // BigQuery has limits on how many snapshots/clones a single table can have and seems to miscount leading to failure when creating too many snapshots from single table
                    // For snapshot table test we use a different source table everytime
                    onBigQuery("CREATE TABLE " + regionCopy + " AS SELECT * FROM tpch.region");
                    onBigQuery("CREATE SNAPSHOT TABLE " + tableName + " CLONE " + regionCopy);
                },
                tableName -> {
                    onBigQuery("DROP SNAPSHOT TABLE " + tableName);
                    onBigQuery("DROP TABLE " + regionCopy);
                });
    }

    private void testEmptyProjection(Consumer<String> createTable, Consumer<String> dropTable)
    {
        // Regression test for https://github.com/trinodb/trino/issues/14981, https://github.com/trinodb/trino/issues/5635 and https://github.com/trinodb/trino/issues/6696
        String name = TEST_SCHEMA + ".test_empty_projection_" + randomNameSuffix();

        createTable.accept(name);
        try {
            assertQuery("SELECT count(*) FROM " + name, "VALUES 5");
            assertQuery("SELECT count(*) FROM " + name, "VALUES 5"); // repeated query to cover https://github.com/trinodb/trino/issues/6696
            assertQuery("SELECT count(*) FROM " + name + " WHERE regionkey = 1", "VALUES 1");
            assertQuery("SELECT count(name) FROM " + name + " WHERE regionkey = 1", "VALUES 1");
        }
        finally {
            dropTable.accept(name);
        }
    }

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair point.

@hashhar hashhar force-pushed the hashhar/fix-flaky-snapshot-bigquery branch from 46d1632 to 9e1b81a Compare July 20, 2023 10:55
This is a refactor to simplify the next commit.
@hashhar hashhar force-pushed the hashhar/fix-flaky-snapshot-bigquery branch from 9e1b81a to c4d0060 Compare July 21, 2023 10:49
@hashhar
Copy link
Member Author

hashhar commented Jul 21, 2023

AC @ebyhr. PTAL again.

BigQuery has limits on how many snapshots or clones can exist for a
single table. It seems BigQuery is miscounting though because we
occasionally see errors like below on CI even though all snapshots get
dropped by their respective tests:

    Caused by: com.google.api.client.googleapis.json.GoogleJsonResponseException: 400 Bad Request
    POST https://www.googleapis.com/bigquery/v2/projects/sep-bq-cicd/queries
    {
      "code": 400,
      "errors": [
        {
          "domain": "global",
          "message": "Cannot have more than 1000 active clones and snapshots of a table. Please use a deep copy, or remove some active clones or snapshots.",
          "reason": "invalid"
        }
      ],
      "message": "Cannot have more than 1000 active clones and snapshots of a table. Please use a deep copy, or remove some active clones or snapshots.",
      "status": "INVALID_ARGUMENT"
    }

A workaround is to use a different table as the source of the snapshot
everytime which is what this change does.
@hashhar hashhar force-pushed the hashhar/fix-flaky-snapshot-bigquery branch from c4d0060 to fe48f2d Compare July 24, 2023 05:53
@hashhar hashhar merged commit 565375b into master Jul 24, 2023
@hashhar hashhar deleted the hashhar/fix-flaky-snapshot-bigquery branch July 24, 2023 07:17
@github-actions github-actions bot added this to the 423 milestone Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bigquery BigQuery connector cla-signed
Development

Successfully merging this pull request may close these issues.

Broken testEmptyProjection in BigQuery connector
4 participants