[GitHub] [beam] prodriguezdefino commented on a diff in pull request #25113: #25112 No longer use GetTable to implement CREATE_IF_NEEDED to avoid low quotas

["prodriguezdefino (via GitHub)" <gi...@apache.org>]

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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org