You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/10/29 08:11:56 UTC
[GitHub] [iceberg] wg1026688210 opened a new pull request #1687: make hive table commit more robust
wg1026688210 opened a new pull request #1687:
URL: https://github.com/apache/iceberg/pull/1687
1. we will seek previous_metadata_location when it occurs File Not Found seek from metadata_location of hive table properties
2 if we used previous_metadata_location on freshing operation ,we will set it to metadata_location in case of both metadataLocation no found when next fresh
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1687: make load hive table more robust
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1687:
URL: https://github.com/apache/iceberg/pull/1687#discussion_r514403724
##########
File path: core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java
##########
@@ -144,27 +145,26 @@ protected String writeNewMetadata(TableMetadata metadata, int newVersion) {
return newMetadataLocation.location();
}
- protected void refreshFromMetadataLocation(String newLocation) {
- refreshFromMetadataLocation(newLocation, null, 20);
+ protected boolean refreshFromMetadataLocation(String newLocation, String previousLocation) {
+ return refreshFromMetadataLocation(newLocation, previousLocation, null, 20);
}
- protected void refreshFromMetadataLocation(String newLocation, int numRetries) {
- refreshFromMetadataLocation(newLocation, null, numRetries);
+ protected boolean refreshFromMetadataLocation(String newLocation, String previousLocation, int numRetries) {
+ return refreshFromMetadataLocation(newLocation, previousLocation, null, numRetries);
}
- protected void refreshFromMetadataLocation(String newLocation, Predicate<Exception> shouldRetry,
- int numRetries) {
+ protected boolean refreshFromMetadataLocation(
Review comment:
iceberg style is to put the new line only before the arguments that don't fit within the line width. So
foo( x, y,
z)
Not
foo (x,
y,
z)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #1687: make load hive table more robust
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on a change in pull request #1687:
URL: https://github.com/apache/iceberg/pull/1687#discussion_r514403724
##########
File path: core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java
##########
@@ -144,27 +145,26 @@ protected String writeNewMetadata(TableMetadata metadata, int newVersion) {
return newMetadataLocation.location();
}
- protected void refreshFromMetadataLocation(String newLocation) {
- refreshFromMetadataLocation(newLocation, null, 20);
+ protected boolean refreshFromMetadataLocation(String newLocation, String previousLocation) {
+ return refreshFromMetadataLocation(newLocation, previousLocation, null, 20);
}
- protected void refreshFromMetadataLocation(String newLocation, int numRetries) {
- refreshFromMetadataLocation(newLocation, null, numRetries);
+ protected boolean refreshFromMetadataLocation(String newLocation, String previousLocation, int numRetries) {
+ return refreshFromMetadataLocation(newLocation, previousLocation, null, numRetries);
}
- protected void refreshFromMetadataLocation(String newLocation, Predicate<Exception> shouldRetry,
- int numRetries) {
+ protected boolean refreshFromMetadataLocation(
Review comment:
iceberg style is to put the new line only before the arguments that don't fit within the line width. So
```
foo( x, y,
z)
```
Not
```
foo (x,
y,
z)
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on pull request #1687: make load hive table more robust
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on pull request #1687:
URL: https://github.com/apache/iceberg/pull/1687#issuecomment-718880328
I have some general concerns before looking more closely, one thing that this PR would do is double the time to failure on misconfiguration which is already quite long. I'm also a little confused about the use case for this PR, are the two locations both equally valid? It seems like we don't wouldn't normally want to fall back to different location?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] wg1026688210 closed pull request #1687: make load hive table more robust
Posted by GitBox <gi...@apache.org>.
wg1026688210 closed pull request #1687:
URL: https://github.com/apache/iceberg/pull/1687
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] wg1026688210 commented on pull request #1687: make load hive table more robust
Posted by GitBox <gi...@apache.org>.
wg1026688210 commented on pull request #1687:
URL: https://github.com/apache/iceberg/pull/1687#issuecomment-719444845
hi~ @RussellSpitzer thanks for your concerns .
I do some improvments on trying previous_location use quite a long time on a new commit .
The parsing metadata retry will stop when it throws NotFoundException.
about the use case for this PR
I found the metadata_location file was deleted when commit fail as I mentioned in #1688 ,
but the metadata_location property of the hive table still became the metadata_location which is deleted.
Flink job fail to restart from checkpoint all the time due to the deleted file
https://github.com/apache/iceberg/blob/4ba48be69963fded08bcd9d0a28e14202191585d/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java#L201-L204
I want to make a improvment that trying the previous_metadata_location only if geting the metadata_location throws NotFoundException, and if the metadata_location is no found , I will set the previous_metadata_location value to it in case of both metadata_location are not found.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] wg1026688210 commented on a change in pull request #1687: make load hive table more robust
Posted by GitBox <gi...@apache.org>.
wg1026688210 commented on a change in pull request #1687:
URL: https://github.com/apache/iceberg/pull/1687#discussion_r514709601
##########
File path: core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java
##########
@@ -144,27 +145,26 @@ protected String writeNewMetadata(TableMetadata metadata, int newVersion) {
return newMetadataLocation.location();
}
- protected void refreshFromMetadataLocation(String newLocation) {
- refreshFromMetadataLocation(newLocation, null, 20);
+ protected boolean refreshFromMetadataLocation(String newLocation, String previousLocation) {
+ return refreshFromMetadataLocation(newLocation, previousLocation, null, 20);
}
- protected void refreshFromMetadataLocation(String newLocation, int numRetries) {
- refreshFromMetadataLocation(newLocation, null, numRetries);
+ protected boolean refreshFromMetadataLocation(String newLocation, String previousLocation, int numRetries) {
+ return refreshFromMetadataLocation(newLocation, previousLocation, null, numRetries);
}
- protected void refreshFromMetadataLocation(String newLocation, Predicate<Exception> shouldRetry,
- int numRetries) {
+ protected boolean refreshFromMetadataLocation(
Review comment:
ok
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org