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 2022/09/01 14:46:57 UTC

[GitHub] [iceberg] gaborkaszab opened a new pull request, #5688: WiP: Make the number of metadata refresh retries configurable

gaborkaszab opened a new pull request, #5688:
URL: https://github.com/apache/iceberg/pull/5688

   For some catalog implementations the number of retries for the metadata
   refresh is hard-coded to 20. For more flexibility this patch adds
   support for adjusting this providing a catalog level property.
   
   TODO: This patch covers JDBCCatalog only. Other Catalog implementations
   should be taken care.
   TODO: This patch doesn't have tests so far.


-- 
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: issues-unsubscribe@iceberg.apache.org

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] rdblue commented on pull request #5688: WiP: Make the number of metadata refresh retries configurable

Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5688:
URL: https://github.com/apache/iceberg/pull/5688#issuecomment-1237290631

   I just merged the alternative fix, so I'm going to close this. Thanks, @gaborkaszab!


-- 
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: issues-unsubscribe@iceberg.apache.org

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] rdblue closed pull request #5688: WiP: Make the number of metadata refresh retries configurable

Posted by GitBox <gi...@apache.org>.
rdblue closed pull request #5688: WiP: Make the number of metadata refresh retries configurable
URL: https://github.com/apache/iceberg/pull/5688


-- 
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: issues-unsubscribe@iceberg.apache.org

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] szehon-ho commented on pull request #5688: WiP: Make the number of metadata refresh retries configurable

Posted by GitBox <gi...@apache.org>.
szehon-ho commented on PR #5688:
URL: https://github.com/apache/iceberg/pull/5688#issuecomment-1234565058

   Yea, I personally think it would be great to make this configurable, as maybe change the default 20 as I feel it is a bit much.
   
   What do you think to put it in TableProperties?  Just going over CatalogProperties and TableProperties, I feel TableProperties has more similar configs like commit.retry.num-retries.  How about in TableProperties :  read.retries.metadata-refresh?
   
   On CatalogProperties there's 'table-default' and 'table-override' that will allow default/override for all flag to be set on Catalog as well, so it will accomplish what this pr tries to do as well.
   
   Another question is whether all catalog will support it.  I think all can, but not sure RestCatalog, cc @rdblue if there's concerns adding such a flag in either Catalog/TableProperties for that.


-- 
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: issues-unsubscribe@iceberg.apache.org

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] gaborkaszab commented on pull request #5688: WiP: Make the number of metadata refresh retries configurable

Posted by GitBox <gi...@apache.org>.
gaborkaszab commented on PR #5688:
URL: https://github.com/apache/iceberg/pull/5688#issuecomment-1237845088

   Thanks for the explanation, @rdblue ! Anyway would it still make sense to make the number of retries configurable instead of being hardcoded to 20?


-- 
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: issues-unsubscribe@iceberg.apache.org

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] rdblue commented on pull request #5688: WiP: Make the number of metadata refresh retries configurable

Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5688:
URL: https://github.com/apache/iceberg/pull/5688#issuecomment-1235950628

   @gaborkaszab, @szehon-ho, I think there's a better way than controlling the number of retries. I doubt anyone is going to actually set this until after they are burned by it. I think the solution is to fail if the retry is because the file doesn't exist. I opened a draft PR here: https://github.com/apache/iceberg/pull/5696
   
   The only problem with that is how to handle cases where you want to retry because of eventual consistency. However, we already handle eventual consistency using read-after-write consistency. To ensure we get read-after-write consistency in S3, we always call `overwrite` since we know metadata locations are unique (contain a random UUID). See https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java#L160-L162


-- 
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: issues-unsubscribe@iceberg.apache.org

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