rdblue commented on a change in pull request #1465:
URL: https://github.com/apache/iceberg/pull/1465#discussion_r490437323
##########
File path:
core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java
##########
@@ -289,19 +311,24 @@ int readVersionHint() {
return Integer.parseInt(in.readLine().replace("\n", ""));
} catch (Exception e) {
- LOG.warn("Error reading version hint file {}", versionHintFile, e);
+ LOG.debug("Error reading version hint file {}", versionHintFile, e);
Review comment:
I think @jacques-n has a good point. The version file should not be
corrupt and we should make sure of it by changing how we write the file. Since
this is for HDFS, creating the file with an atomic rename to ensure the entire
file is written before making it the current version hint makes sense to me.
That way we don't get dirty reads.
Doing that would result in cases like @fbocse brings up: we'd have to delete
the version hint file to rename into its place. And there are cases where the
hint file is missing, like when trying to load a table to check whether it
exists. In that case, I don't think that it makes sense to warn when a hint
file is missing, only when it is corrupt. It would still be good to have an
info log when the file is missing and there is no metadata (table doesn't
exist), and a warning if the file is missing but a metadata file is found.
Last, I'm for an option to throw an exception when the hint file is corrupt
rather than throwing a warning, but the hint file is not intended as a
requirement for correctness, so I'd turn this off by default.
----------------------------------------------------------------
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]
