Rename the Snapshot Retention policy

[flyrain]

I'm proposing to rename the table maintenance policy "snapshot-retention" to "snapshot-expiry". While "snapshot-retention" is a reasonable name, it's a bit too general. I believe "snapshot-expiry" more precisely captures the intent of the policy, which is specifically about removing expired snapshots.

I think this is the right time to make the change before we include the policy schemas in an upcoming release.

Will change related classes in a followup PR.

[singhpk234]

LGTM as well, Thanks @flyrain !

[dimas-b]

As for me, given the example below, the term "retention" looks more relevant. Expiry means something is not valid/usable anymore, while the purpose of this policy (although unstated) appears to be controlling when snapshots are discarded, while still being logically valid.

(edit: removed "minor", let's discuss)

[snazy]

Right.
Retention and expiration are two different things.
The term retention is more correct for this use case.

[eric-maynard]

It's a little ambiguous. It seems like the intent of the policy is to determine under what conditions expiry should happen. Similarly, we talk about masking policies, or compaction policies. Expiry seems marginally more in line with this, while retention describes the state when no maintenance action is taken.

Maybe the contention is around the exact definition of "expire"? If it's interpreted as something that happens passively, such as when a policy becomes too old, then I agree calling this an expiry policy doesn't make as much sense. I was interpreting expire to be used in the same way that Iceberg uses it, where "expire snapshots" is an action wherein you remove certain snapshots. The Iceberg docs say, for example:

Expiring old snapshots removes them from metadata

[dimas-b]

Is the behaviour of this policy defined somewhere? (sorry, if I'm missing the obvious). It would help a lot in reasoning about names and descriptions... TBH, the current description is rather obscure (also properties is just a bag of arbitrary entries).

[flyrain]

The example below demonstrates a free-form config map—its key-value pairs can be anything, like expire-in-days. So we shouldn't base a strong argument on this example alone. Using retention here isn't technically wrong, it's a valid term. However, the main reason for renaming it to expiry is to make the intent of this table maintenance policy more precise: it's specifically about removing expired snapshots. Retention is a broader concept and could imply that the maintenance job does more than just snapshot cleanup.

[dimas-b]

@flyrain : Could you clarify what "removing expired snapshots" involves and what makes snapshots "expired" (ideally in the description of the policy we're renaming in this PR)?

[flyrain]

Let me add it here first:
What are involved?

1. A table with snapshots
2. A Polaris policy which indicates the snapshot expiry is enabled for the table.
3. Table configuration or policy config indicates how to expire snapshots, including parameters like max age of a snapshot.
4. An engine or client to able to remove expired snapshots, clean up files for the table.

[dimas-b]

Thanks for the update @flyrain .

With that description the term "expiry" makes sense to me.

However, I believe it would be nice to have that information in the description of the policy (in code). Referring back to GH discussions is not practical.

[eric-maynard]

Yeah I have definitely been a little confused about the semantics of policies. Big +1 on better descriptions (in code)