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/02/05 20:15:49 UTC

[GitHub] [incubator-iceberg] yathindranath opened a new pull request #780: expire table metadata with ExpireSnapshots

yathindranath opened a new pull request #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780
 
 
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#discussion_r379948715
 
 

 ##########
 File path: core/src/test/java/org/apache/iceberg/TestTables.java
 ##########
 @@ -218,7 +244,7 @@ public OutputFile newOutputFile(String path) {
 
     @Override
     public void deleteFile(String path) {
-      if (!new File(path).delete()) {
+      if (new File(path).exists() && !new File(path).delete()) {
 
 Review comment:
   Nit: should not create a file from the path twice.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] yathindranath opened a new pull request #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
yathindranath opened a new pull request #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780
 
 
   This PR takes care of the issue #181 and is follow-up of #631.
   
   1. Adds new method `expireMetadataRetainLast` to `ExpireSnapshots`.
   2. If `expireMetadataRetainLast(n)` is called,  retains at least `n` latest metadata files and deletes rest.
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-600749322
 
 
   Since we want to build actions for things like cleaning up orphan files, we can use this utility class there.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] yathindranath commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
yathindranath commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-596280702
 
 
   @rdblue any thoughts or suggestions? 

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue edited a comment on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue edited a comment on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-586777788
 
 
   I think the purpose of this PR is to implement [a comment](https://github.com/apache/incubator-iceberg/pull/631#issuecomment-554168010) I made on #631:
   
   > I think if we add expiration during ExpireSnapshots, then that's when the number of metadata versions would be pruned. For example, expiration might keep 10 previous metadata file references. Between expiration calls, the ones tracked continues to grow.
   
   I think we need to have the ability to delete old metadata files as new ones are created, or else tables with lots of transactions will suffer from larger and larger metadata files between snapshot expiration runs. But unlike when we were discussing this on the last PR, I'm not so sure that we need to clean up old files during snapshot expiration. It would be nice to do it, but we don't want metadata file contents to grow too large because of this.
   
   Right now, I'm leaning toward not doing this cleanup during snapshot expiration. It's additional complexity for not much benefit. What do you think, @yathindranath, @aokolnychyi, and @arina-ielchiieva?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-600759190
 
 
   @yathindranath, that sounds good to me. I'll close this one.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-598812854
 
 
   @yathindranath, I don't think it's a good idea to include this in snapshot expiration, though I could be convinced otherwise. To keep this moving forward, let's start by adding a separate utility (maybe `MetadataUtil.clean`?) that traverses the old metadata lists to clean them up. Once we have that, it would be fairly easy to add that to snapshot expiration if we think it's a good idea.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587142773
 
 
   The main problem with removing old metadata versions in snapshot expiration is that the number of old metadata references in the metadata file can grow very large. How would we keep that from being a performance problem for tables if we only support removing old metadata files with expire snapshots?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587159479
 
 
   Yes, the maximum limits the growth. But if we remove references to old metadata without deleting it, then files are still leaked. The purpose of keeping a list of old metadata files is to track and delete them.
   
   That leak is why the approach I suggested in the other PR doesn't work. By the time expire snapshots runs, references may have been removed without deleting the files. That, or you let the list grow large with a huge maximum and probably hit performance problems. The only way to avoid the list growing large _and_ avoid leaking files is to enable cleanup after commit.
   
   And if we recommend cleanup after commit, then what is the value of cleaning up while expiring snapshots? I don't see much value in this because we can't reasonably get rid of the other cleanup method.
   
   I think the best path forward is to default `write.metadata.delete-after-commit.enabled` to `true`.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] aokolnychyi edited a comment on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
aokolnychyi edited a comment on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587184268
 
 
   > Yes, the maximum limits the growth. But if we remove references to old metadata without deleting it, then files are still leaked. The purpose of keeping a list of old metadata files is to track and delete them.
   
   Will dropped files be still referenced in old metadata JSON files? Whenever we expire snapshots, the list of previous files in the current version file might not have all of the old files we need to clean up. But what if we load some older files? We should be able to trace of them, right? (not sure about the complexity)
   
   We definitely want to avoid the situation when the number of old metadata JSON files grows beyond a threshold as we've been in that situation before manifest lists.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue closed pull request #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue closed pull request #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780
 
 
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] yathindranath edited a comment on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
yathindranath edited a comment on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587130697
 
 
   @rdblue The default value of `TableProperties.METADATA_DELETE_AFTER_COMMIT_ENABLED` is false and if we enable this at a later point, can leave orphan metadata files. I believe, it's better to do clean up of old metadata files during snapshot expiration and remove the current logic from the commit what we added in #631.
   
   @aokolnychyi I agree, we can have table properties for snapshot retention interval, number snapshots to retain and number of metadata files to retain. We can create a different PR for expiration with table properties.
   
   Sorry, closed the PR accidentally, reopened it.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] yathindranath commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
yathindranath commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587213861
 
 
   As @aokolnychyi mentioned, we should be able to trace all of the files to delete by reading few metadata files. This PR follows the above approach to list all the files to delete.
   
   May be we could deprecate `write.metadata.delete-after-commit.enabled` property and if `write.metadata.previous-versions-max` is defined, always cleanup after commit and no clean up while expiring snapshots.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] aokolnychyi edited a comment on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
aokolnychyi edited a comment on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587184268
 
 
   > Yes, the maximum limits the growth. But if we remove references to old metadata without deleting it, then files are still leaked. The purpose of keeping a list of old metadata files is to track and delete them.
   
   Will dropped files be still referenced in old metadata JSON files? Whenever we expire snapshots, the list of previous files in the current version file might not have all of the old files we need to clean up. But what if we load some older files? We should be able to trace all of the files to delete, right? (not sure about the complexity)
   
   We definitely want to avoid the situation when the number of old metadata JSON files grows beyond a threshold as we've been in that situation before manifest lists.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587184268
 
 
   > Yes, the maximum limits the growth. But if we remove references to old metadata without deleting it, then files are still leaked. The purpose of keeping a list of old metadata files is to track and delete them.
   
   Will dropped files be still referenced in old metadata JSON files? Whenever we expire snapshots, the list of files in the current version file might not have all of the old files we need to clean up. But what if we load some older files? We should be able to trace of them, right? (not sure about the complexity)
   
   We definitely want to avoid the situation when the number of old metadata JSON files grows beyond a threshold as we've been in that situation before manifest lists.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] yathindranath commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
yathindranath commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587147438
 
 
   @rdblue `TableProperties.METADATA_PREVIOUS_VERSIONS_MAX` limits the references of previous metadata files in the metadata file and won't grow very large after reaching max. The default value is 100 and it only keeps references of previous 100 metadata files at max.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587233741
 
 
   > Will dropped files be still referenced in old metadata JSON files?
   
   Yes, but do we want to open lots of metadata files to get the set of files to delete? I'm assuming that we don't want to open multiple files to do this.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-586777788
 
 
   I think the purpose of this PR is to implement [a comment](https://github.com/apache/incubator-iceberg/pull/631#issuecomment-554168010) I made on #631:
   
   > I think if we add expiration during ExpireSnapshots, then that's when the number of metadata versions would be pruned. For example, expiration might keep 10 previous metadata file references. Between expiration calls, the ones tracked continues to grow.
   
   The open question is how this should behave. I'm not sure I agree with what I suggested the last time, with separate configurations for number to keep when cleaning up during expiration and for the maximum number to keep when writing metadata. If possible, it would be better to have just one configuration and let users choose whether to delete old metadata files as new ones are written (the current implementation) or to delete old metadata files during snapshot expiration.
   
   I think we need to have the ability to delete old metadata files as new ones are created, or else tables with lots of transactions will suffer from larger and larger metadata files between snapshot expiration runs. I'm not so sure that we need to clean up old files during snapshot expiration. It would be nice to do it, but we don't want metadata file contents to grow too large because of this.
   
   Right now, I'm leaning toward not doing this cleanup during snapshot expiration. It's additional complexity for not much benefit. What do you think, @yathindranath, @aokolnychyi, and @arina-ielchiieva?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-583044458
 
 
   I think this PR is a follow-up on #631. 
   
   As far as I remember, we planned that the main place to remove old metadata JSON files will be while expiring snapshots. After a quick look, it seems we won't delete metadata files unless the user explicitly calls a method in `ExpireSnapshots`. I would, probably, always reduce the number of metadata JSON files to a smaller number and expose a method to overwrite the number of files to keep. For example, we can have a table property that defines the default number of files to keep while expiring snapshots and a method to override this value programmatically.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] yathindranath closed pull request #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
yathindranath closed pull request #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780
 
 
   

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] yathindranath commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
yathindranath commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-600394238
 
 
   @rdblue sounds good, shall I close this PR then?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-583044654
 
 
   FYI @arina-ielchiieva 

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] yathindranath commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
yathindranath commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587130697
 
 
   The default `TableProperties.METADATA_DELETE_AFTER_COMMIT_ENABLED` is false, if we enable this at a later point can leave orphan metadata files. I believe, it's better to do clean up of old metadata files during snapshot expiration and remove the current logic from the commit what we added in #631 
   
   @aokolnychyi we can have table properties for snapshot retention interval, number snapshots to retain

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] rdblue edited a comment on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
rdblue edited a comment on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-586777788
 
 
   I think the purpose of this PR is to implement [a comment](https://github.com/apache/incubator-iceberg/pull/631#issuecomment-554168010) I made on #631:
   
   > I think if we add expiration during ExpireSnapshots, then that's when the number of metadata versions would be pruned. For example, expiration might keep 10 previous metadata file references. Between expiration calls, the ones tracked continues to grow.
   
   The open question is how this should behave. I'm not sure I agree with what I suggested the last time, with separate configurations for number to keep when cleaning up during expiration and for the maximum number to keep when writing metadata. If possible, it would be nice to have just one configuration and let users choose whether to delete old metadata files as new ones are written (the current implementation) or to delete old metadata files during snapshot expiration.
   
   I think we need to have the ability to delete old metadata files as new ones are created, or else tables with lots of transactions will suffer from larger and larger metadata files between snapshot expiration runs. I'm not so sure that we need to clean up old files during snapshot expiration. It would be nice to do it, but we don't want metadata file contents to grow too large because of this.
   
   Right now, I'm leaning toward not doing this cleanup during snapshot expiration. It's additional complexity for not much benefit. What do you think, @yathindranath, @aokolnychyi, and @arina-ielchiieva?

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [incubator-iceberg] aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots

Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on issue #780: expire table metadata with ExpireSnapshots
URL: https://github.com/apache/incubator-iceberg/pull/780#issuecomment-587249903
 
 
   > Yes, but do we want to open lots of metadata files to get the set of files to delete? 
   
   It is a bit unclear whether that will become a problem as we don't have to open every previous metadata file, it is enough to open only the last referenced. We will read 1 per 100 metadata files under the default configuration.
   
   Having said that, I don't have a strong argument/opinion here. I want us to explore all options as it seems nice to have this logic if it does not complicate things or affect performance.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org