-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Docs: Add gc.enabled table property #14676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: nk1506 <[email protected]>
kevinjqliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM minor comment on phrasing
Here's the definition of gc.enabled property:
iceberg/core/src/main/java/org/apache/iceberg/TableProperties.java
Lines 349 to 350 in a3c538f
| public static final String GC_ENABLED = "gc.enabled"; | |
| public static final boolean GC_ENABLED_DEFAULT = true; |
And where its used
iceberg/core/src/main/java/org/apache/iceberg/CatalogUtil.java
Lines 119 to 125 in a3c538f
boolean gcEnabled = PropertyUtil.propertyAsBoolean(metadata.properties(), GC_ENABLED, GC_ENABLED_DEFAULT); if (gcEnabled) { // delete data files only if we are sure this won't corrupt other tables deleteFiles(io, manifestsToDelete); } iceberg/core/src/main/java/org/apache/iceberg/RemoveSnapshots.java
Lines 87 to 89 in a3c538f
ValidationException.check( PropertyUtil.propertyAsBoolean(base.properties(), GC_ENABLED, GC_ENABLED_DEFAULT), "Cannot expire snapshots: GC is disabled (deleting files may corrupt other tables)");
docs/docs/configuration.md
Outdated
| | history.expire.max-snapshot-age-ms | 432000000 (5 days) | Default max age of snapshots to keep on the table and all of its branches while expiring snapshots | | ||
| | history.expire.min-snapshots-to-keep | 1 | Default min number of snapshots to keep on the table and all of its branches while expiring snapshots | | ||
| | history.expire.max-ref-age-ms | `Long.MAX_VALUE` (forever) | For snapshot references except the `main` branch, default max age of snapshot references to keep while expiring snapshots. The `main` branch never expires. | | ||
| | gc.enabled | true | Disable garbage collection operations such as expiring snapshots or removing orphan files | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| | gc.enabled | true | Disable garbage collection operations such as expiring snapshots or removing orphan files | | |
| | gc.enabled | true | Allows garbage collection operations such as expiring snapshots and removing orphan files | |
nit: using "Disable" here is confusing, esp the property name has "enabled" and defaults to true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: please change the PR description accordingly.
Co-authored-by: Kevin Liu <[email protected]>
|
Thanks @ebyhr and thanks everyone for the review |
Continuation of #9231