TheNeuralBit commented on code in PR #17428:
URL: https://github.com/apache/beam/pull/17428#discussion_r855330571
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiDynamicDestinationsTableRow.java:
##########
@@ -95,7 +96,14 @@ public MessageConverter<T> getMessageConverter(
+ "using a create disposition of CREATE_IF_NEEDED.");
}
}
+ } else {
+ // Make sure we register this schema with the cache, unless there's
already a more
+ // up-to-date schema.
+ tableSchema =
+ MoreObjects.firstNonNull(
+ SCHEMA_CACHE.putSchemaIfAbsent(tableReference, tableSchema),
tableSchema);
Review Comment:
Is this change to fix some exception that was raising in the daemon and
crashing it?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableSchemaCache.java:
##########
@@ -271,10 +280,11 @@ public void refreshThread() {
if (timeRemaining.getMillis() > 0) {
Thread.sleep(timeRemaining.getMillis());
}
- } catch (InterruptedException e) {
- runUnderMonitor(() -> this.stopped = true);
- return;
- } catch (IOException e) {
+ } catch (Exception e) {
Review Comment:
Is it appropriate to swallow _all_ exceptions? Could there be cases where
the daemon needs to be monitored and restarted?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/TableSchemaCache.java:
##########
@@ -271,10 +280,11 @@ public void refreshThread() {
if (timeRemaining.getMillis() > 0) {
Thread.sleep(timeRemaining.getMillis());
}
- } catch (InterruptedException e) {
- runUnderMonitor(() -> this.stopped = true);
- return;
- } catch (IOException e) {
+ } catch (Exception e) {
+ // Since this is a daemon thread, don't exit until it is explicitly shut
down. Exiting early
+ // can cause the
+ // pipeline to stall.
+ LOG.error("Caught exception: " + e);
Review Comment:
Should this include some message about the context in which the exception
was raised?
--
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]
