allenpradeep commented on a change in pull request #12010:
URL: https://github.com/apache/beam/pull/12010#discussion_r441173677
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java
##########
@@ -1373,10 +1375,22 @@ public void processElement(ProcessContext c) {
@VisibleForTesting
static class WriteToSpannerFn extends DoFn<Iterable<MutationGroup>, Void> {
- private transient SpannerAccessor spannerAccessor;
private final SpannerConfig spannerConfig;
private final FailureMode failureMode;
+ // Only create one SpannerAccessor for each different SpannerConfig.
Review comment:
Setup for SpannerAccessor are done from BatchSpannerRead,
CreateTransactionFn, NaiveSpannerRead, ReadSpannerSchema.
Should this go into SpannerAccessor directly and when create() is called,
should that take care of creating if absent rather than SpannerIO? let me know
your thoughts.
##########
File path:
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java
##########
@@ -1423,18 +1438,46 @@ public void processElement(ProcessContext c) {
}
@Setup
- public void setup() throws Exception {
- // set up non-serializable values here.
- spannerAccessor = SpannerAccessor.create(spannerConfig);
+ public void setup() {
+ spannerAccessor = spannerAccessors.get(spannerConfig);
+ if (spannerAccessor == null) {
+ synchronized (spannerAccessors) {
Review comment:
I guess this could could be common code in SpannerAccessor(may be part
of SpannerAccessor.create()) and could be shared as SpannerAccessor is called
from other places as well.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
