khaledh opened a new issue, #40189: URL: https://github.com/apache/beam/issues/40189
### What would you like to happen? `TableRowToStorageApiProto.messageFromMap` and `messageFromTableRow` take an `allowMissingRequiredFields` parameter. When it is `true`, the conversion already skips the per-field missing-required check. It then calls `DynamicMessage.Builder.build()`, which runs a full recursive `isInitialized()` walk over the message tree and throws `UninitializedMessageException` if any required field is unset. That has two consequences: 1. **Cost.** The walk visits every field of every nested message on every record. On a production Dataflow pipeline writing rows with 1,059 schema fields and nesting depth 10 through `BigQueryIO.write()` with `STORAGE_WRITE_API` and `withAutoSchemaUpdate(true)`, the destination descriptor is built with no required fields, so the walk can never fail, yet it still runs. JFR attributed a large share of the conversion step's CPU and allocation (descriptor list wrappers and iterators) to it. 2. **Semantics.** The parameter's promise is kept during conversion and broken at build time. A caller that passes `true` with a descriptor that still has required fields gets the missing-field exception from `build()` anyway. ### Proposal At both `build()` sites: ```java return allowMissingRequiredFields ? builder.buildPartial() : builder.build(); ``` No new method, no new parameter, no change to the `false` path. Both in-tree callers that pass `true` already use descriptors built without required fields (`StorageApiDynamicDestinationsTableRow.TableRowConverter` when `autoSchemaUpdates` is on, and `AppendClientInfo.encodeUnknownFields`), so for them this is a pure optimization with byte-identical output. **Behavior change to declare:** an out-of-tree caller passing `true` with a descriptor that still has required fields would receive a partial message instead of a conversion-time `UninitializedMessageException`. The BigQuery service still rejects such a row. Given the parameter's name, this is the documented behavior being honored, but it should be called out in `CHANGES.md`. If maintainers prefer, the change can be guarded with a one-time check that the descriptor has no required fields. ### Measurements On the production-shaped workload above, measured locally over a 280-record corpus with production options (five JVM forks, census-weighted): this change alone reduced whole-record CPU by 12.3%. Combined with the descriptor lookup change in #40188, 35.9% locally. The combined pair on x86 Dataflow at equal, fixed capacity (35 × n2-standard-4, Streaming Engine, backlogged input): 33.6% lower CPU per record and 46% higher drain throughput, with exact offset-set parity. ### Related #40188 addresses the other independent cost in the same conversion (per-field `Descriptor.findFieldByName` lookups). The two changes touch different regions of `TableRowToStorageApiProto.java` and can land in either order. A PR implementing this is ready. ### Issue Priority Priority: 2 (default / most feature requests should be filed as P2) ### Issue Components - [ ] Component: Python SDK - [x] Component: Java SDK - [ ] Component: Go SDK - [ ] Component: Typescript SDK - [x] Component: IO connector - [ ] Component: Beam YAML - [ ] Component: Beam examples - [ ] Component: Beam playground - [ ] Component: Beam katas - [ ] Component: Website - [ ] Component: Infrastructure - [ ] Component: Spark Runner - [ ] Component: Flink Runner - [ ] Component: Prism Runner - [ ] Component: Twister2 Runner - [ ] Component: Hazelcast Jet Runner - [ ] Component: Google Cloud Dataflow Runner -- 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]
