pvary commented on a change in pull request #3099:
URL: https://github.com/apache/iceberg/pull/3099#discussion_r716568468
##########
File path:
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveClientPool.java
##########
@@ -110,57 +103,4 @@ public void
testGetTablesFailsForNonReconnectableException() throws Exception {
AssertHelpers.assertThrows("Should throw exception", MetaException.class,
"Another meta exception", () -> clients.run(client ->
client.getTables("default", "t")));
}
-
- @Test
- public void testConnectionFailureRestoreForMetaException() throws Exception {
Review comment:
Having both Iceberg and Hive retry could also be problematic /
surprising. We will just do NxM retries (2x2 in the default case).
I see 2 typical error scenario in case of `lock`:
1. Can not reach the HMS - connection is broken, or something like this -
Retry is a perfect solution
2. The locks are queued, but we have to wait and something happens in the
meantime - Retry will be blocked forever (or until the lock timeout is reached
for the previously queued locks) - Retry is problematic / unnecessary.
How typical is that the first request on the connection is the `lock`? If it
happens often then the 1st situation could happen often. If we usually get the
metadata for the table and then try to lock then the 1st situation is unlikely
to happen.
I would prefer to keep the retry login in one place
(`RetryingMetaStoreClient`) and simplify the Iceberg code by removing the retry
altogether, but I can accept keeping the code as it is - we most probably will
turn off the Iceberg retries in our deployment.
--
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]
