feature: expire snapshots action

[cmcarthur]

Which issue does this PR close?

• Closes Maintenance: Expire Snapshots Action #1454.

What changes are included in this PR?

• Adds ExpireSnapshotsAction with extensive test coverage

Are these changes tested?

I've added the following unit tests covering the happy path logic in the action:

• test_builder_pattern: unit test on the builder pattern
• test_collect_files_to_delete_logic: unit test on collect_files_to_delete
• test_default_configuration: tests defaults on builder pattern
• test_dry_run: asserts that when execute is called in dry run mode, no files are deleted and no transaction is generated or applied

The remaining tests setup various scenarios and call the full action to ensure the correct commit is generated and applied, and the correct files are deleted:

    maintenance::expire_snapshots::tests::test_empty_snapshot_ids_list
    maintenance::expire_snapshots::tests::test_expire_current_snapshot_error
    maintenance::expire_snapshots::tests::test_expire_readonly_table_error
    maintenance::expire_snapshots::tests::test_expire_snapshots_many_snapshots_retain_last
    maintenance::expire_snapshots::tests::test_expire_snapshots_older_than_timestamp
    maintenance::expire_snapshots::tests::test_expire_snapshots_one_snapshot
    maintenance::expire_snapshots::tests::test_expire_snapshots_zero_snapshots
    maintenance::expire_snapshots::tests::test_expire_specific_snapshot_ids
    maintenance::expire_snapshots::tests::test_max_concurrent_deletes_configuration
    maintenance::expire_snapshots::tests::test_nonexistent_snapshot_ids
    maintenance::expire_snapshots::tests::test_retain_last_minimum_validation

[CTTY]

Hi @cmcarthur , thanks for the PR! I really like the overall design!

There may be some tweaks we can use to integrate with the transaction API better. I've put my thoughts in the comments

[CTTY]

I'm planning to add retry logic in tx.commit, so you don't have to retry here

issue: #1387

[CTTY]

I'm thinking of something like this so it leverages the new Transaction code logic:

pub trait ExpireSnapshots: Send + Sync {
    /// Trigger tx.commit, delete files if tx.commit succeeded, and collect ExpireSnapshotsResult
    async fn execute(&self, catalog: &dyn Catalog) -> Result<ExpireSnapshotsResult>;
    fn table(&self) -> Table; // returns the table that this action operates on so ExpireSnapshotsAction doesn't have to hold a Table 
}

impl TransactionAction for ExpireSnapshotsAction {
   async fn commit(self: Arc<Self>, table: &Table) -> Result<ActionCommit> {
    // does the actual snapshots cleaning
    // wraps updates and requirements into ActionCommit for Catalog to update metadata
  }
}

This should help avoid using tx.apply directly (#1455 (comment))

wdyt?

[CTTY]

With the new Transaction API, Transaction should only hold actions (pending actions to commit), updates and requirements should be removed after this PR: #1451

Based on this thought, tx.apply shouldn't be accessible to anything except tx.commit

[cmcarthur]

appreciate the feedback @CTTY -- I'll incorporate these changes and work on adding further tests. thanks!