main and shadow table as a Side Input in spanner-to-sourcedb #2950

sm745052 · 2025-11-03T05:32:36Z

Fixes b/456048565

Added main and shadow table as a Side Input in spanner-to-sourcedb so that the job startup time is less.

Context:
when main and shadow table are given as main input, when there are large number of tables, the startup time exceeded 12 min which is the default startup timeout for a dataflow job.

Earlier:
Using 4k tables: Startup time exceeded 12 min

Now:
Tested 4k tables: Startup time was around 4 min

NOTE:

Also added some TODOs in AssignShardIdFnTests which seem to be a bit off. Will need to correct them in another PR since that is out of scope of this PR.

gemini-code-assist · 2025-11-03T05:32:57Z

Summary of Changes

Hello @sm745052, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant refactoring to the SpannerToSourceDb Dataflow template, focusing on optimizing how DDL (Data Definition Language) and schema mapping objects are managed and distributed across the Beam pipeline. By transitioning from direct object passing to using PCollectionViews for DDL and implementing lazy initialization for schema mappers within the worker-side DoFns, the changes aim to enhance the pipeline's efficiency, scalability, and robustness, particularly in handling large schema definitions.

Highlights

DDL and Schema Mapper Handling: The Data Definition Language (DDL) objects for both the main Spanner database and shadow tables are now fetched eagerly at the pipeline construction phase and converted into PCollectionViews. This allows them to be efficiently distributed as side inputs to Beam workers.
Lazy Initialization: The ISchemaMapper and IShardIdFetcher objects, which depend on the DDL, are now initialized lazily within the processElement method of the AssignShardIdFn and SourceWriterFn DoFns. This ensures that these potentially large objects are created only when needed on each worker, improving serialization and resource management.
Refactored Constructors and Method Signatures: The constructors for AssignShardIdFn and SourceWriterFn, along with several helper methods, have been updated to accept SpannerToSourceDb.Options and PCollectionView<Ddl> instead of direct Ddl and ISchemaMapper instances, aligning with the new side input pattern.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

codecov · 2025-12-01T13:45:33Z

Codecov Report

❌ Patch coverage is 63.36634% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.43%. Comparing base (efb574a) to head (b67e6e2).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...cloud/teleport/v2/templates/SpannerToSourceDb.java	0.00%	17 Missing ⚠️
...rms/SpannerToSourceDbProcessInformationSchema.java	78.84%	9 Missing and 2 partials ⚠️
...eport/v2/templates/transforms/AssignShardIdFn.java	73.68%	3 Missing and 2 partials ⚠️
...v2/templates/transforms/SourceWriterTransform.java	0.00%	3 Missing ⚠️
...leport/v2/templates/transforms/SourceWriterFn.java	90.00%	0 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (63.36%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2950      +/-   ##
============================================
+ Coverage     50.26%   50.43%   +0.16%     
- Complexity     5024     5031       +7     
============================================
  Files           967      970       +3     
  Lines         59271    59524     +253     
  Branches       6459     6492      +33     
============================================
+ Hits          29794    30020     +226     
+ Misses        27376    27375       -1     
- Partials       2101     2129      +28

Components	Coverage Δ
spanner-templates	`70.67% <63.36%> (+0.20%)`	⬆️
spanner-import-export	`69.01% <ø> (ø)`
spanner-live-forward-migration	`79.81% <ø> (+0.12%)`	⬆️
spanner-live-reverse-replication	`77.58% <63.36%> (+0.47%)`	⬆️
spanner-bulk-migration	`88.21% <ø> (-0.12%)`	⬇️

Files with missing lines	Coverage Δ
...leport/v2/templates/transforms/SourceWriterFn.java	`83.67% <90.00%> (-0.13%)`	⬇️
...v2/templates/transforms/SourceWriterTransform.java	`0.00% <0.00%> (ø)`
...eport/v2/templates/transforms/AssignShardIdFn.java	`80.22% <73.68%> (+0.99%)`	⬆️
...rms/SpannerToSourceDbProcessInformationSchema.java	`78.84% <78.84%> (ø)`
...cloud/teleport/v2/templates/SpannerToSourceDb.java	`3.11% <0.00%> (+3.11%)`	⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

manitgupta

It always a great practice to add details of the testing done in the PR description! ;)
Please be as elaborate as you want to.

sm745052 · 2025-12-02T09:10:56Z

It always a great practice to add details of the testing done in the PR description! ;) Please be as elaborate as you want to.

Addressed

manitgupta · 2025-12-02T09:48:09Z

...nner-to-sourcedb/src/main/java/com/google/cloud/teleport/v2/templates/SpannerToSourceDb.java

+    PCollectionTuple ddlTuple =
+        pipeline.apply(
+            "Process Information Schema",
+            new SpannerToSourceDbProcessInformationSchema(


PTransform names should end in *Transform to reflect that they are PTransforms. Should be named to reflect the action that the class is doing: SpannerInformationSchemaProcessorTransform.

manitgupta · 2025-12-02T09:50:01Z

...nner-to-sourcedb/src/main/java/com/google/cloud/teleport/v2/templates/SpannerToSourceDb.java

  }

-  private static ISchemaMapper getSchemaMapper(Options options, Ddl ddl) {
+  public static ISchemaMapper getSchemaMapper(Options options, Ddl ddl) {


Why did the access modifier change here?

manitgupta · 2025-12-02T09:50:58Z

...ourcedb/src/main/java/com/google/cloud/teleport/v2/templates/transforms/AssignShardIdFn.java

  @Teardown
  public void teardown() {
-    if (spannerConfig != null) {
+    if (spannerAccessor != null) {


What was breaking earlier?

it was not breaking but this seems more logical

manitgupta · 2025-12-02T09:52:13Z

...ourcedb/src/main/java/com/google/cloud/teleport/v2/templates/transforms/AssignShardIdFn.java

+    if (this.schemaMapper == null) {
+      SpannerToSourceDb.Options options =
+          c.getPipelineOptions().as(SpannerToSourceDb.Options.class);
+      this.schemaMapper = SpannerToSourceDb.getSchemaMapper(options, ddl);
+    }


This seems like an anti-pattern and is very confusing.

@ProcessElement is called for every element. This code would result in a schemaMapper being initialized at the time of processing an element. Logically, schema mapping should be initialized prior to processing elements.

You should not be importing SpannerToSourceDb here. This looks like an inverted import.

Overall the code flow seems very odd to me. Please take a look.

manitgupta · 2025-12-02T09:56:33Z

...sourcedb/src/main/java/com/google/cloud/teleport/v2/templates/transforms/SourceWriterFn.java

+    if (this.schemaMapper == null) {
+      SpannerToSourceDb.Options options =
+          c.getPipelineOptions().as(SpannerToSourceDb.Options.class);
+      this.schemaMapper = SpannerToSourceDb.getSchemaMapper(options, ddl);
+    }


Same comment as earlier.

manitgupta · 2025-12-02T09:57:38Z

...b/src/main/java/com/google/cloud/teleport/v2/templates/transforms/SourceWriterTransform.java

+                        this.ddlView,
+                        this.shadowTableDdlView))
+                .withSideInputs(ddlView, shadowTableDdlView)


Why is ddlView and shadowTableDdlView passed both as a parameter and as a SideInput? Won't the sideInput already be available inside the DoFn?

we need a reference for the sideinput which we get via the constructor. LMK if i am missing something.

manitgupta · 2025-12-02T09:58:48Z

...google/cloud/teleport/v2/templates/transforms/SpannerToSourceDbProcessInformationSchema.java

+
+  @Override
+  public PCollectionTuple expand(PBegin p) {
+    return p.apply("Create empty", Create.of((Void) null))


This is a confusing name. Root transforms are generally called Pulse or something similar.

manitgupta · 2025-12-02T10:01:09Z

...google/cloud/teleport/v2/templates/transforms/SpannerToSourceDbProcessInformationSchema.java

+                .withOutputTags(MAIN_DDL_TAG, TupleTagList.of(SHADOW_TABLE_DDL_TAG)));
+  }
+
+  static class ProcessInformationSchemaFn extends DoFn<Void, Ddl> {


Nit: My personal preference is to have a DoFns which non-trivial amount of logic into their own class. It makes the PTransform using these DoFns easy to extend in the future without increasing the class length too much.

manitgupta · 2025-12-02T10:43:13Z

...google/cloud/teleport/v2/templates/transforms/SpannerToSourceDbProcessInformationSchema.java

+
+    @ProcessElement
+    public void processElement(ProcessContext c) {
+      LOG.info("Starting processing of Information Schema...");


Is this needed? Let's add logs so that they add value when something goes wrong..

Sandeep Mishra added 2 commits November 3, 2025 01:06

added ddls as sideinput ion spanner-to-sourcedb

24389d9

spotless

470ff22

sm745052 requested a review from a team as a code owner November 3, 2025 05:32

sm745052 requested review from darshan-sj and shreyakhajanchi November 3, 2025 05:32

pull-request-size bot added the size/XL label Nov 3, 2025

sm745052 marked this pull request as draft November 3, 2025 07:40

sm745052 added 4 commits November 28, 2025 09:24

removed unnecessary comments

9d1cb1e

added process information schema as a Transform

b2ba18f

merge and updated new tests

b3e2b9f

added tests and spotless

77b90d3

sm745052 added the bug-fix label Dec 1, 2025

sm745052 requested a review from manitgupta December 1, 2025 12:01

updated tests

9261ba9

updated tests and added TODOs in probably wrong code

b67e6e2

sm745052 marked this pull request as ready for review December 1, 2025 16:43

manitgupta reviewed Dec 2, 2025

View reviewed changes

sm745052 changed the title ~~Sp2src sideview~~ main and shadow table as a Side Input in spanner-to-sourcedb Dec 2, 2025

sm745052 closed this Dec 2, 2025

sm745052 reopened this Dec 2, 2025

manitgupta reviewed Dec 2, 2025

View reviewed changes

main and shadow table as a Side Input in spanner-to-sourcedb #2950

Are you sure you want to change the base?

main and shadow table as a Side Input in spanner-to-sourcedb #2950

Uh oh!

Conversation

sm745052 commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Nov 3, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

codecov bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

manitgupta left a comment

Choose a reason for hiding this comment

Uh oh!

sm745052 commented Dec 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sm745052 commented Nov 3, 2025 •

edited

Loading

codecov bot commented Dec 1, 2025 •

edited

Loading