prodriguezdefino commented on code in PR #25113:
URL: https://github.com/apache/beam/pull/25113#discussion_r1083545037


##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/CreateTableHelpers.java:
##########
@@ -46,8 +48,28 @@ public class CreateTableHelpers {
    */
   private static Set<String> createdTables = Collections.newSetFromMap(new 
ConcurrentHashMap<>());
 
+  // When CREATE_IF_NEEDED is specified, BQ tables should be created if they 
do not exist. This
+  // method detects
+  // errors on table operations, and attempts to create the table if necessary.
+  static void createTableWrapper(Callable<Void> action, Callable<Boolean> 
tryCreateTable)
+      throws Exception {
+    for (int retry = 0; retry <= 1; ++retry) {
+      try {
+        action.call();
+      } catch (ApiException | StatusRuntimeException e) {
+        // TODO: Once BigQuery reliably returns a consistent error on table 
not found, we should

Review Comment:
   Maybe an issues can be created in the client to track the feature request / 
improvement. 



##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/CreateTableHelpers.java:
##########
@@ -46,8 +48,28 @@ public class CreateTableHelpers {
    */
   private static Set<String> createdTables = Collections.newSetFromMap(new 
ConcurrentHashMap<>());
 
+  // When CREATE_IF_NEEDED is specified, BQ tables should be created if they 
do not exist. This
+  // method detects
+  // errors on table operations, and attempts to create the table if necessary.
+  static void createTableWrapper(Callable<Void> action, Callable<Boolean> 
tryCreateTable)
+      throws Exception {
+    for (int retry = 0; retry <= 1; ++retry) {
+      try {
+        action.call();
+      } catch (ApiException | StatusRuntimeException e) {
+        // TODO: Once BigQuery reliably returns a consistent error on table 
not found, we should
+        // only try creating
+        // the table on that error.
+        boolean created = tryCreateTable.call();

Review Comment:
   not sure about this but, should this backoff before retrying? more 
considering that we are requesting the table info from BQ before sending the 
create request.
   
   I think we would want to resolve the table creation as fast as possible but 
as many bundle instances will try to do the same, almost at the same time, 
maybe backoff could save some create request if we wait some amount of time on 
some bundles (not sure, maybe randomly?)



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