pvary commented on a change in pull request #1465:
URL: https://github.com/apache/iceberg/pull/1465#discussion_r490852704



##########
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.
   
   I think that is something for another PR.
   
   > 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.
   
   Here is my proposed solution:
   - No metadata directory: DEBUG log - there are some perfectly valid cases 
where we expect that the directory/table is not there. Higher level log message 
could be very confusing (Seen very bad examples in Hive code 😢)
   - Existing metadata directory, no hint file: WARN log - transient error, or 
hint file error. Good to be aware of.
   - IOException while checking for existence of the metadata directory or 
listing: WARN log - Some fs level exception. Has to be checked.
   
   Your thoughts?
   
   > 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.
   
   Could someone please point me a place (code lines) where and how Iceberg 
configurations are handled, so I can use them correctly?
   
   Thanks,
   Peter
   




----------------------------------------------------------------
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]

Reply via email to