damccorm commented on code in PR #37433:
URL: https://github.com/apache/beam/pull/37433#discussion_r2743654856
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java:
##########
@@ -2109,10 +2109,9 @@ private static Dialect getDialect(SpannerConfig
spannerConfig, PipelineOptions p
// Allow passing the credential from pipeline options to the getDialect()
call.
SpannerConfig spannerConfigWithCredential =
buildSpannerConfigWithCredential(spannerConfig, pipelineOptions);
- try (SpannerAccessor sa =
SpannerAccessor.getOrCreate(spannerConfigWithCredential)) {
- DatabaseClient databaseClient = sa.getDatabaseClient();
- return databaseClient.getDialect();
- }
+ SpannerAccessor sa =
SpannerAccessor.getOrCreate(spannerConfigWithCredential);
+ DatabaseClient databaseClient = sa.getDatabaseClient();
+ return databaseClient.getDialect();
Review Comment:
Is there a reason to undo this change? It seems like it is still probably
better to force the SpannerAccessor class to close
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -614,7 +614,7 @@ class BeamModulePlugin implements Plugin<Project> {
def google_clients_version = "2.0.0"
def google_cloud_bigdataoss_version = "2.2.26"
// [bomupgrader] determined by: com.google.cloud:google-cloud-spanner,
consistent with: google_cloud_platform_libraries_bom
- def google_cloud_spanner_version = "6.104.0"
+ def google_cloud_spanner_version = "6.107.0"
Review Comment:
Do we need this version to be tracked at all now or can we remove this line?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java:
##########
@@ -265,6 +272,14 @@ static SpannerOptions buildSpannerOptions(SpannerConfig
spannerConfig) {
builder.setCredentials(credentials.get());
}
+ ValueProvider<java.time.Duration> waitForSessionCreationDuration =
+ spannerConfig.getWaitForSessionCreationDuration();
+ java.time.Duration waitDuration =
Optional.ofNullable(waitForSessionCreationDuration)
+ .map(ValueProvider::get)
+ .orElse(DEFAULT_SESSION_WAIT_DURATION);
+ builder.setSessionPoolOption(
+
SessionPoolOptions.newBuilder().setWaitForMinSessionsDuration(waitDuration).build());
Review Comment:
Is this change related to the hangs?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java:
##########
@@ -113,6 +117,9 @@ public static SpannerAccessor getOrCreate(SpannerConfig
spannerConfig) {
static SpannerOptions buildSpannerOptions(SpannerConfig spannerConfig) {
SpannerOptions.Builder builder = SpannerOptions.newBuilder();
+ // Disable gRPC gcp extension which was causing the application thread to
stall.
+ builder.disableGrpcGcpExtension();
Review Comment:
Is the root cause known/is there a timeline for us to remove this? If so,
could you please file a Beam issue tracking the fix and reference it in a TODO
here?
--
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]
