[AMORO-4048] Saving cleanup opeartion process info in table_process

[zhangwl9]

Why are the changes needed?

Close #4048.

Brief change log

How was this patch tested?

• Add some test cases that check the changes thoroughly including negative and positive cases if possible

• Add screenshots for manual tests if appropriate

• Run test locally before making a pull request

Documentation

• Does this pull request introduce a new feature? (yes / no)
• If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.59459% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 28.84%. Comparing base (fda105e) to head (5779284).
⚠️ Report is 24 commits behind head on master.

Files with missing lines	Patch %	Lines
...amoro/server/scheduler/PeriodicTableScheduler.java	94.59%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #4077      +/-   ##
============================================
+ Coverage     22.39%   28.84%   +6.44%     
- Complexity     2552     3958    +1406     
============================================
  Files           458      656     +198     
  Lines         42116    52462   +10346     
  Branches       5917     6644     +727     
============================================
+ Hits           9433    15133    +5700     
- Misses        31871    36229    +4358     
- Partials        812     1100     +288     

Flag	Coverage Δ
core	28.84% <94.59%> (?)
trino	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR addresses [AMORO-4048] by persisting cleanup executor run information into the table_process table, similar to how optimizing executions are tracked.

Changes:

• Add persistence logic in PeriodicTableScheduler to insert/update table_process records for cleanup operations (RUNNING → SUCCESS/FAILED).
• Update inline scheduler tests to validate cleanup process persistence and expose testing hooks.
• Add a new test covering cleanup process status transitions and stored failure messages.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
amoro-ams/src/main/java/org/apache/amoro/server/scheduler/PeriodicTableScheduler.java	Adds creation and completion persistence for cleanup processes in table_process.
amoro-ams/src/test/java/org/apache/amoro/server/scheduler/inline/PeriodicTableSchedulerTestBase.java	Exposes test-only wrappers to call the new persistence methods.
amoro-ams/src/test/java/org/apache/amoro/server/scheduler/inline/TestPeriodicTableSchedulerCleanup.java	Adds a persistence-focused test validating cleanup process lifecycle in table_process.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

SnowflakeIdGenerator is instantiated per PeriodicTableScheduler instance. Because the generator’s uniqueness relies on per-instance sequence state (and defaults to machineId=0), multiple schedulers generating IDs within the same time slice can produce duplicate processIds and violate the table_process.process_id PK. Use a shared/static generator (or a centralized ID service) so all schedulers share sequence state, or pass distinct machine IDs per instance.

Suggested change

	private final SnowflakeIdGenerator idGenerator = new SnowflakeIdGenerator();
	private static final SnowflakeIdGenerator ID_GENERATOR = new SnowflakeIdGenerator();

[Copilot]

The catch (Throwable t) block records the error for persistence but never logs it. This will silently swallow failures when execute() or the persistence calls throw, making cleanup issues hard to diagnose in production. Please add an error log (including table identifier and cleanup operation) and consider rethrowing Errors if you don’t want to mask fatal JVM problems.

Suggested change

	executionError = t;
	executionError = t;
	logger.error(
	"Failed to execute cleanup operation {} for table {}",
	cleanupOperation,
	tableRuntime.getTableIdentifier(),
	t);
	if (t instanceof Error) {
	throw (Error) t;
	}

[Copilot]

fail_message in table_process is constrained (e.g., Postgres has length(fail_message) <= 4096). Persisting Throwable.getMessage() without truncation can violate this constraint (and also loses stack trace detail). Consider using the existing org.apache.amoro.utils.ExceptionUtil.getErrorMessage(t, 4000) pattern used by optimizing to both capture stack traces and bound the stored length.

[Copilot]

createCleanupProcessInfo / persistCleanupResult are marked @VisibleForTesting but exposed as public methods on a production scheduler base class. This widens the API surface unnecessarily; making them protected (or package-private) would still allow test subclasses to access them while reducing accidental external use.

[Copilot]

The inner helper is named PersistencyHelper, while the surrounding package/classes use persistence terminology (e.g., PersistentBase, server.persistence). Renaming to PersistenceHelper would improve consistency and avoid confusion when searching for persistence-related utilities.

[Copilot]

This assertion can be flaky: createTime and finishTime are both derived from System.currentTimeMillis() and may be equal when the persistence call happens within the same millisecond (or due to DB timestamp precision). Consider asserting <= instead, or introducing a small deterministic delay to guarantee finishTime is after createTime.

Suggested change

	Assert.assertTrue(persistedSuccess.getCreateTime() < persistedSuccess.getFinishTime());
	Assert.assertTrue(persistedSuccess.getCreateTime() <= persistedSuccess.getFinishTime());