thiagotnunes commented on code in PR #24390:
URL: https://github.com/apache/beam/pull/24390#discussion_r1035481624


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/mapper/ChangeStreamRecordMapper.java:
##########
@@ -83,30 +90,41 @@ public class ChangeStreamRecordMapper {
   private static final String CHILD_PARTITIONS_COLUMN = "child_partitions";
   private static final String PARENT_PARTITION_TOKENS_COLUMN = 
"parent_partition_tokens";
   private static final String TOKEN_COLUMN = "token";
+  private final Dialect dialect;
+  private final JsonFormat.Printer printer;
+  private final JsonFormat.Parser parser;
 
-  ChangeStreamRecordMapper() {}
+  ChangeStreamRecordMapper(Dialect dialect) {
+    this.dialect = dialect;
+
+    this.printer =

Review Comment:
   I think not, we create a new mapper in the 
[Setup](https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/dofn/ReadChangeStreamPartitionDoFn.java#L186-L187)
 method, which is called per SDF, so we (the implementers) won't call it 
concurrently, but I don't know if the backend will (Windmill in the case of 
Dataflow). I would think not.
   
   It would be good to test this under load to verify we don't get any errors



-- 
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]

Reply via email to