lawofcycles commented on code in PR #3320:
URL: https://github.com/apache/iceberg-python/pull/3320#discussion_r3524186649


##########
pyiceberg/table/__init__.py:
##########
@@ -1036,17 +1074,82 @@ def commit_transaction(self) -> Table:
             The table with the updates applied.
         """
         if len(self._updates) > 0:
-            self._requirements += 
(AssertTableUUID(uuid=self.table_metadata.table_uuid),)
-            self._table._do_commit(  # pylint: disable=W0212
-                updates=self._updates,
-                requirements=self._requirements,
+            properties = self._table.metadata.properties
+            num_retries_val = property_as_int(
+                properties, TableProperties.COMMIT_NUM_RETRIES, 
TableProperties.COMMIT_NUM_RETRIES_DEFAULT
+            )
+            num_retries = num_retries_val if num_retries_val is not None else 
TableProperties.COMMIT_NUM_RETRIES_DEFAULT
+            min_wait_val = property_as_int(
+                properties, TableProperties.COMMIT_MIN_RETRY_WAIT_MS, 
TableProperties.COMMIT_MIN_RETRY_WAIT_MS_DEFAULT
+            )
+            min_wait_ms = min_wait_val if min_wait_val is not None else 
TableProperties.COMMIT_MIN_RETRY_WAIT_MS_DEFAULT
+            max_wait_val = property_as_int(
+                properties, TableProperties.COMMIT_MAX_RETRY_WAIT_MS, 
TableProperties.COMMIT_MAX_RETRY_WAIT_MS_DEFAULT
+            )
+            max_wait_ms = max_wait_val if max_wait_val is not None else 
TableProperties.COMMIT_MAX_RETRY_WAIT_MS_DEFAULT
+            total_timeout_val = property_as_int(
+                properties, TableProperties.COMMIT_TOTAL_RETRY_TIME_MS, 
TableProperties.COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT
             )
+            total_timeout_ms = (
+                total_timeout_val if total_timeout_val is not None else 
TableProperties.COMMIT_TOTAL_RETRY_TIME_MS_DEFAULT
+            )
+            start_time = time.monotonic()
+            self._requirements += 
(AssertTableUUID(uuid=self.table_metadata.table_uuid),)
+
+            try:
+                for attempt in range(num_retries + 1):
+                    try:
+                        self._table._do_commit(  # pylint: disable=W0212
+                            updates=self._updates,
+                            requirements=self._requirements,
+                        )
+                        self._cleanup_uncommitted_manifests()
+                        break
+                    except CommitFailedException:
+                        elapsed_ms = (time.monotonic() - start_time) * 1000
+                        if attempt == num_retries or not 
self._snapshot_producers or elapsed_ms >= total_timeout_ms:
+                            raise
+
+                        wait = min(min_wait_ms * (2**attempt), max_wait_ms)
+                        jitter = random.uniform(0, 0.25 * wait)

Review Comment:
   addressed.



##########
pyiceberg/table/__init__.py:
##########
@@ -1036,17 +1074,82 @@ def commit_transaction(self) -> Table:
             The table with the updates applied.
         """
         if len(self._updates) > 0:
-            self._requirements += 
(AssertTableUUID(uuid=self.table_metadata.table_uuid),)
-            self._table._do_commit(  # pylint: disable=W0212
-                updates=self._updates,
-                requirements=self._requirements,
+            properties = self._table.metadata.properties
+            num_retries_val = property_as_int(
+                properties, TableProperties.COMMIT_NUM_RETRIES, 
TableProperties.COMMIT_NUM_RETRIES_DEFAULT
+            )
+            num_retries = num_retries_val if num_retries_val is not None else 
TableProperties.COMMIT_NUM_RETRIES_DEFAULT
+            min_wait_val = property_as_int(
+                properties, TableProperties.COMMIT_MIN_RETRY_WAIT_MS, 
TableProperties.COMMIT_MIN_RETRY_WAIT_MS_DEFAULT
+            )
+            min_wait_ms = min_wait_val if min_wait_val is not None else 
TableProperties.COMMIT_MIN_RETRY_WAIT_MS_DEFAULT

Review Comment:
   addressed.



##########
pyiceberg/table/__init__.py:
##########
@@ -1036,17 +1074,82 @@ def commit_transaction(self) -> Table:
             The table with the updates applied.
         """
         if len(self._updates) > 0:
-            self._requirements += 
(AssertTableUUID(uuid=self.table_metadata.table_uuid),)
-            self._table._do_commit(  # pylint: disable=W0212
-                updates=self._updates,
-                requirements=self._requirements,
+            properties = self._table.metadata.properties
+            num_retries_val = property_as_int(
+                properties, TableProperties.COMMIT_NUM_RETRIES, 
TableProperties.COMMIT_NUM_RETRIES_DEFAULT
+            )
+            num_retries = num_retries_val if num_retries_val is not None else 
TableProperties.COMMIT_NUM_RETRIES_DEFAULT

Review Comment:
   addressed.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to