Make entity drop and cleanup task creation atomic

[iting0321]

Summary

Implement the

// TODO: Add a way to create the task entities atomically with dropping the entity;
// in the meantime, if the server fails partway through a dropEntity, it's possible that
// the entity is dropped but we don't have any persisted task records that will carry
// out the cleanup.

Entity cleanup currently requires two persistence operations:

1. Drop the original entity.
2. Create the cleanup task.

A crash between these operations can leave a dropped resource without a cleanup task. This change introduces BasePersistence.deleteEntityAndCreateEntities(...) so both changes are committed or rolled back together.

Changes

Atomically drop entities and create cleanup tasks across TreeMap and JDBC, while preserving idempotency. Add rollback tests verifying that task creation failures leave the original entity intact and create no new tasks.

Checklist

• 🛡️ Don't disclose security issues! (contact security@apache.org)
• 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
• 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
• 💡 Added comments for complex logic
• 🧾 Updated CHANGELOG.md (if needed)
• 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

[Copilot]

Pull request overview

Implements an atomic persistence operation to drop an entity and create associated cleanup task entities in a single commit/rollback unit, addressing the crash window between “drop” and “schedule cleanup”.

Changes:

• Add BasePersistence.deleteEntityAndCreateEntities(...) and implement it for transactional persistence and JDBC persistence.
• Update AtomicOperationMetaStoreManager to use the new atomic operation when scheduling entity cleanup tasks during drops.
• Add rollback tests (TreeMap + JDBC) to verify that cleanup task creation failures do not leave the entity dropped or create partial tasks.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java	Adds shared assertion verifying rollback behavior when cleanup-task creation fails.
polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisTreeMapAtomicOperationMetaStoreManagerTest.java	Wires the new rollback assertion into the TreeMap atomic metastore test suite.
polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/AbstractTransactionalPersistence.java	Implements deleteEntityAndCreateEntities using a single transactional action.
polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java	Introduces the new atomic delete+create persistence API and documents its semantics.
polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java	Uses the new atomic operation to make drop + cleanup task scheduling transactional for atomic-operation persistence.
persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java	Wires the new rollback assertion into the JDBC atomic metastore test suite.
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java	Adds JDBC implementation of the new atomic delete+create API.
persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NonFunctionalBasePersistence.java	Adds the new method to the non-functional implementation with the existing “use metastore manager” exception behavior.

[jbonofre]

This is a good PR that fixes correctness.

I would appreciate your thoughts on my comment regarding DropEntityResult.

[jbonofre]

This is a behavior change here, as it can now escape dropEntityIfExists.

Previously, the code ignored the result of createEntityIfNotExists, so a name collision on the cleanup task silently "succeeded".
Now, EntityAlreadyExistsException propagates.
I think it's more "correct", but:

1. It's inconsistent with this class's convention of translating EntityAlreadyExistsException into a result status. We should consider returning a DropEntityResult error status instead of throwing a raw persistence exception through the manager API.
2. The realistic trigger is a concurrent double-drop of the same entity (two cleanup tasks named entityCleanup_<sameId> with different task ids). The earlier existence check narrows this window but doesn't fully close it. It's worth confirming the REST/service layer degrades gracefully (maps to 409, not 500) rather than leaking a stack trace.