rdblue commented on a change in pull request #1429:
URL: https://github.com/apache/iceberg/pull/1429#discussion_r485094755
##########
File path:
core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java
##########
@@ -75,7 +76,14 @@ public TableMetadata refresh() {
try {
doRefresh();
} catch (NoSuchTableException e) {
- LOG.warn("Could not find the table during refresh, setting current
metadata to null", e);
+ // Adjust log level according to the type of exception, as it might be
intentional to call
+ // the method without determining the type of table in prior (like
SparkSessionCatalog),
+ // and in such case it may not be an error case if the table is not an
Iceberg table.
+ if (e instanceof NoSuchIcebergTableException) {
Review comment:
To answer most of @kbendick's questions: this is extended by
`HiveTableOperations`. It is also the recommended way to support a [custom
metastore](http://iceberg.apache.org/custom-catalog/), which is what we do at
Netflix.
The reason why this warning exists is to catch the case where a table is
deleted concurrently. The Hive implementation throws the exception when
[current metadata is not null but the table doesn't
exist](https://github.com/apache/iceberg/blob/454101c3573acb9cd94d6d9a306ed99a5a324ed9/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L125).
The table instance needs to be in a consistent state if it is reused for a
create, which is why we set the metadata to null. At the time, we considered
`NoSuchTableException` to be more rare than it is with the wrapped catalogs.
I think a better solution is to log the warning only when the table
previously had metadata. What we could do is check before calling `doRefresh`
whether `currentMetadata` is null. If it is not, then we want to log the
warning. Otherwise, we should let it go through. We always want to re-throw
because the caller is responsible for handling the exception. This is supposed
to be helpful context.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
