Merlin-S3NS commented on PR #11649:
URL: https://github.com/apache/seatunnel/pull/11649#issuecomment-5393350867
Thank you very much for this comprehensive re-review and for verifying the
call chain analysis! I'm glad we were able to align on the framework-level
placeholder resolution mechanism and the multi-table SPI contracts.
I have addressed all remaining feedback items in the latest commit and
updated the PR description:
---
### 1. The Netty and Protobuf Shading Relocation Details (`pom.xml`)
As requested, here is the exact technical breakdown and justification for
the shading changes in `connector-bigquery/pom.xml`:
#### The Protobuf Problem (Why `com.google.protobuf` relocation was
removed):
* **What Happened**: In Google Cloud's BigQuery Storage Write API
(`google-cloud-bigquerystorage`), generated Protobuf classes
(`com.google.cloud.bigquery.storage.v1.ProtoRows`,
`com.google.protobuf.Descriptors`, `com.google.protobuf.DynamicMessage`)
interact dynamically with `com.google.protobuf` core classes.
* **The Crash**: When Maven Shade plugin blindly relocated
`com.google.protobuf` to `org.apache.seatunnel.shade.com.google.protobuf`, it
shaded core Protobuf classes, but JNI / gRPC stub compiled bytecode inside
`google-cloud-bigquerystorage` still expected standard
`com.google.protobuf.Message` on the classloader. At runtime during row
serialization, this caused fatal errors:
```text
java.lang.ClassCastException: com.google.protobuf.Descriptors$Descriptor
cannot be cast to
org.apache.seatunnel.shade.com.google.protobuf.Descriptors$Descriptor
```
* **How We Solved It**: Removing the `<relocation>` for
`com.google.protobuf` allows `google-cloud-bigquerystorage` to use the
unshaded, unified Protobuf library provided by Google Cloud BOM (`26.72.0`),
resolving gRPC proto row serialization crashes.
#### The Netty Problem (Why `io.netty` relocation was added):
* **What Happened**: The BigQuery Storage Write API relies on gRPC
(`grpc-netty-shaded`) over Netty for high-throughput HTTP/2 streaming
connections to BigQuery endpoints (`bigquery.googleapis.com` or
`bigquery.s3nsapis.fr`).
* **The Crash**: On execution engines (Flink, Spark, or SeaTunnel Engine),
the host JVM carries an older version of Netty (e.g., Netty 4.1.42 vs Netty
4.1.100 required by Google's gRPC transport). Without shading, the JVM
classloader loaded the host engine's older `io.netty.handler.codec.http2`
classes. At runtime, during channel handshake to BigQuery, gRPC threw fatal
linkage errors:
```text
java.lang.NoSuchMethodError:
io.netty.handler.codec.http2.Http2Headers.intensity()
```
* **How We Solved It**: Adding
`<relocation><pattern>io.netty</pattern></relocation>` isolates BigQuery's gRPC
HTTP/2 transport into `${seatunnel.shade.package}.io.netty`, completely
shielding BigQuery streaming writes from engine-level Netty version conflicts.
---
### 2. Incompatible Changes Documentation (Issue 1)
* **Action Taken**: Added an entry to
[`docs/en/introduction/concepts/incompatible-changes.md`](https://github.com/apache/seatunnel/blob/dev/docs/en/introduction/concepts/incompatible-changes.md)
under **Connector Changes**:
> **Breaking Change: BigQuery Sink Connector — default schema save mode
introduces automatic table creation**
> - **Affected component**: `seatunnel-connectors-v2/connector-bigquery`
> - **Description**: The BigQuery sink connector (`connector-bigquery`)
now implements `SupportSaveMode` with support for `schema_save_mode` and
`data_save_mode`. The default `schema_save_mode` is set to
`CREATE_SCHEMA_WHEN_NOT_EXIST`.
> - **Impact**: Upgrading existing pipelines targeting a non-existent
table will now automatically create the table in BigQuery with the source
schema instead of failing fast at the BigQuery API layer.
> - **Migration Guide**: To preserve the legacy fail-fast behavior,
explicitly configure `schema_save_mode = "ERROR_WHEN_SCHEMA_NOT_EXIST"` in your
BigQuery sink configuration.
---
### 3. Refined Exception Checks (Issue 3)
* **Action Taken**: Refactored `BigQuerySaveModeHandler.java` to use direct
`e instanceof BigQueryException` checks for Google Cloud SDK exceptions before
catching general fallback warnings.
---
### 4. Code Maintenance Comment
* **Action Taken**: Added an explanatory comment in `BigQuerySink.java`
noting that `TABLE_ID` is pre-resolved per target table by
`TablePlaceholderProcessor` during `FactoryUtil.createAndPrepareSink()`.
---
### Verification Summary
- **JUnit Unit Tests**: `138 / 138 PASSED` (`BUILD SUCCESS`)
- **Code Formatting**: `mvn spotless:apply` (100% compliant)
Thank you again for the fantastic collaboration!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
