szehon-ho commented on a change in pull request #3099:
URL: https://github.com/apache/iceberg/pull/3099#discussion_r715796353
##########
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:
Good idea, done. Put the test back.
Side note: the flexibility of making it configurable is nice, and is a
potential solution to the issue I mentioned, I am tempted to set it to true in
HiveClientOperations::acquireLock in this line.
```
LockResponse lockResponse = metaClients.run(client ->
client.lock(lockRequest));
```
I can see from Hive point of view why they don't want to auto-retry, but
most exception scenarios are not going to be in the corner case of having
succeeded in the backend and leaving the lock. In all honesty, if the Iceberg
job fails because lock() throws TTransportException, end user will just try
again but it is a wasted effort. Not sure if any thoughts on it?
cc @pvary
--
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]
