Add file loads streaming integration tests #28312

ahmedabu98 · 2023-09-05T18:17:42Z

Addressing this large BigQuery integration testing gap.

Includes streaming with single load jobs, copy jobs, auto-sharding/fixed shards, and dynamic destinations.
Limiting it to test only ~half the cases with Dataflow runner.

Uncovered an issue with dynamic destinations while doing this: #28309

ahmedabu98 · 2023-09-06T14:36:54Z

Fixes #28309

ahmedabu98 · 2023-09-12T14:16:47Z

Run PostCommit_Java_Dataflow

ahmedabu98 · 2023-09-12T14:17:44Z

R: @Abacn
R: @johnjcasey
R: @shunping-google

github-actions · 2023-09-12T14:19:40Z

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

Abacn

Thanks! this will fill an important gap, had comment below

Abacn · 2023-09-12T17:05:32Z

...a/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BatchLoads.java

-            .apply("GroupByKey", GroupByKey.create())
-            .apply("Extract Values", Values.create())
+            // We use this and the following GBK to aggregate by final destination.
+            // This way, each destination has its own pane sequence


I see this fixes #28309, thanks!

Abacn · 2023-09-12T17:06:31Z

...o/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryUtils.java

@@ -689,6 +689,10 @@ public static Row toBeamRow(Schema rowSchema, TableSchema bqSchema, TableRow jso
      }
    }

+    if (jsonBQValue instanceof byte[] && fieldType.getTypeName() == TypeName.BYTES) {


just curious - what does this short cut return do?

This type (bye[]) was previously unsupported so converting a TableRow with byte[] field to Beam Row would throw an UnsupportedOperationException below.

Abacn · 2023-09-12T17:22:59Z

...e-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/FileLoadsStreamingIT.java

+        "Checking there is no duplication", expectedBeamRows.size(), actualBeamRows.size());
+  }
+
+  @Test


It may not be necessary to run a full matrix of every variable (fixed shard, dynamical destination, use copyJobs) which generates 2*2*2 possibilities.

A choice to reduce the complexity is test default + each flag in a test + test all feature added, this reduces the complexity from 2^n to n+2. Also, given the default case (had autoshard) will be tested in #28272 (serve as default), I propose keep the following one:

testLoadWithFixedShards

testWithAutoShardingAndCopyJobs

testDynamicDestinationsWithAutoSharding

testDynamicDestinationsWithAutoShardingAndCopyJobs

So we're saying that fixed/dynamic sharding is independent from single/multi loads. I think I agree with that.

I mostly agree with that new list, I just think we should do testDynamicDestinationsWithFixedShards instead of testDynamicDestinationsWithAutoSharding because there is a slightly different code path there

P.S. I think we can remove the FILE_LOADS test in #28272 and keep FileLoadsStreamingIT as the hub for streaming loads tests. That way we can keep the other file as BigQueryIOStorageWriteIT for storage write. wdyt?

we should do testDynamicDestinationsWithFixedShards instead of testDynamicDestinationsWithAutoSharding

Sure that's good

...e-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/FileLoadsStreamingIT.java

ahmedabu98

Thanks for the feedback @Abacn! I reduced test complexity, PTAL

ahmedabu98 · 2023-09-12T17:32:52Z

...o/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryUtils.java

@@ -689,6 +689,10 @@ public static Row toBeamRow(Schema rowSchema, TableSchema bqSchema, TableRow jso
      }
    }

+    if (jsonBQValue instanceof byte[] && fieldType.getTypeName() == TypeName.BYTES) {


This type (bye[]) was previously unsupported so converting a TableRow with byte[] field to Beam Row would throw an UnsupportedOperationException below.

...e-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/FileLoadsStreamingIT.java

ahmedabu98 · 2023-09-12T17:53:34Z

...e-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/FileLoadsStreamingIT.java

+        "Checking there is no duplication", expectedBeamRows.size(), actualBeamRows.size());
+  }
+
+  @Test


So we're saying that fixed/dynamic sharding is independent from single/multi loads. I think I agree with that.

I mostly agree with that new list, I just think we should do testDynamicDestinationsWithFixedShards instead of testDynamicDestinationsWithAutoSharding because there is a slightly different code path there

ahmedabu98 · 2023-09-12T18:04:37Z

...e-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/FileLoadsStreamingIT.java

+        "Checking there is no duplication", expectedBeamRows.size(), actualBeamRows.size());
+  }
+
+  @Test


P.S. I think we can remove the FILE_LOADS test in #28272 and keep FileLoadsStreamingIT as the hub for streaming loads tests. That way we can keep the other file as BigQueryIOStorageWriteIT for storage write. wdyt?

ahmedabu98 · 2023-09-12T18:11:07Z

Run PostCommit_Java_Dataflow

Abacn · 2023-09-12T19:32:49Z

P.S. I think we can remove the FILE_LOADS test in #28272 and keep FileLoadsStreamingIT as the hub for streaming loads tests.

Sounds good to me

ahmedabu98 · 2023-09-13T13:11:56Z

Most recent Legacy runner post commits passed
Direct Runner tests passed as well

ahmedabu98 · 2023-09-13T13:16:44Z

Failing tests are unrelated

Abacn · 2023-09-13T15:31:46Z

Postcommit failure was due to quota "Startup of the worker pool in zone us-central1-b failed to bring up any of the desired 1 workers."

Good to check - is the test specified specific zone? As we are bringing up more tests, we should request increasing quota and/or distribute works in different region.

ahmedabu98 · 2023-09-13T15:33:24Z

is the test specified specific zone

No specific zone for these tests, they all default to US multi-region

file loads streaming integration tests

4c4c6d3

github-actions bot added java io gcp labels Sep 5, 2023

ahmedabu98 added 2 commits September 5, 2023 16:04

spotless

40c5794

spotless

bff7e46

ahmedabu98 added 5 commits September 6, 2023 15:42

fix dynamic destinations copy jobs

fac0b7e

with schema

9be89f7

less temp load jobs

e9f0a4f

nits

1508cd2

disable for runnerV2 until pane index is fixed

bf25ea1

github-actions bot added runners dataflow labels Sep 11, 2023

ahmedabu98 marked this pull request as ready for review September 12, 2023 14:17

Abacn reviewed Sep 12, 2023

View reviewed changes

reduce number of tests

315327b

ahmedabu98 commented Sep 12, 2023

View reviewed changes

add streaming engine experiment for legacy dataflow runner

610ccb1

Abacn mentioned this pull request Sep 12, 2023

Fix pane info in BigQuery load job id #28272

Closed

3 tasks

johnjcasey approved these changes Sep 13, 2023

View reviewed changes

Abacn approved these changes Sep 13, 2023

View reviewed changes

ahmedabu98 merged commit 1032533 into apache:master Sep 13, 2023
18 of 20 checks passed

ahmedabu98 mentioned this pull request Sep 19, 2023

[Bug]: [Java BQ FILE_LOADS] When streaming to dynamic destinations with copy jobs and CREATE_IF_NEEDED, only the first destination's table is created #28309

Closed

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add file loads streaming integration tests #28312

Add file loads streaming integration tests #28312

ahmedabu98 commented Sep 5, 2023 •

edited

Loading

ahmedabu98 commented Sep 6, 2023

ahmedabu98 commented Sep 12, 2023

ahmedabu98 commented Sep 12, 2023

github-actions bot commented Sep 12, 2023

Abacn left a comment

Abacn Sep 12, 2023

Abacn Sep 12, 2023

ahmedabu98 Sep 12, 2023

Abacn Sep 12, 2023 •

edited

Loading

ahmedabu98 Sep 12, 2023

ahmedabu98 Sep 12, 2023

Abacn Sep 13, 2023

ahmedabu98 left a comment

ahmedabu98 Sep 12, 2023

ahmedabu98 Sep 12, 2023

ahmedabu98 Sep 12, 2023

ahmedabu98 commented Sep 12, 2023

Abacn commented Sep 12, 2023

ahmedabu98 commented Sep 13, 2023

ahmedabu98 commented Sep 13, 2023

Abacn commented Sep 13, 2023

ahmedabu98 commented Sep 13, 2023

Add file loads streaming integration tests #28312

Add file loads streaming integration tests #28312

Conversation

ahmedabu98 commented Sep 5, 2023 • edited Loading

ahmedabu98 commented Sep 6, 2023

ahmedabu98 commented Sep 12, 2023

ahmedabu98 commented Sep 12, 2023

github-actions bot commented Sep 12, 2023

Abacn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abacn Sep 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahmedabu98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahmedabu98 commented Sep 12, 2023

Abacn commented Sep 12, 2023

ahmedabu98 commented Sep 13, 2023

ahmedabu98 commented Sep 13, 2023

Abacn commented Sep 13, 2023

ahmedabu98 commented Sep 13, 2023

ahmedabu98 commented Sep 5, 2023 •

edited

Loading

Abacn Sep 12, 2023 •

edited

Loading