szehon-ho commented on a change in pull request #3099:
URL: https://github.com/apache/iceberg/pull/3099#discussion_r717837774
##########
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:
@pvary yea this PR disables the retry for Hive case, but leaves an
option for turning it on to a particular caller, to address your concern.
About lock(), thanks for the reply, maybe you are right that it is not
typical case to have TTransportException especially if getMetadata has already
succeeded on same connection.
The only thing bothering me is that there is no way for the client to
distinguish case 1 and 2 based on exception type, and thus nothing they can do
really except retry. There is a ShowLock call, and potentially they can check
if there is a queued lock from the failed call, is it it is their own , and if
so then unlock it. But I'm not sure how reliable this is. Anyway its maybe
for another review then.
--
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]
