Skip to content

[#13983] feat(tms): Register policy timeTrigger on the expand scheduler - #13897

Open
lasdf1234 wants to merge 16 commits into
apache:branch-tms-schedulerfrom
lasdf1234:feature/tms-policy-timetrigger-scheduler
Open

lasdf1234 wants to merge 16 commits into
apache:branch-tms-schedulerfrom
lasdf1234:feature/tms-policy-timetrigger-scheduler

Conversation

@lasdf1234

@lasdf1234 lasdf1234 commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

What changes were proposed in this pull request?

Follow-up on the TMS scheduler pools from Epic #13760.

When a maintenance policy carries timeTrigger, create, alter, enable, disable, and delete keep a matching policy-expand row in scheduled_expand_tasks:

  • Enabled policy with a non-blank cron: insert or move the row. The task instance id is the policy id, and the due time is the next Quartz cron fire time.
  • Disabled policy, blank timeTrigger, or a successful delete: remove the row.
  • Non-maintenance policies are ignored.

The expand handler is still a no-op. This change only registers the row. The db-scheduler client forces auto-commit, and the pools commit when auto-commit is off, so writes survive the entity-store connection pool.

Why are the changes needed?

Policy content stores timeTrigger, but nothing was writing that schedule into db-scheduler. Without this, a created or updated maintenance policy never becomes an expand lease, and a deleted or disabled policy would leave a stale row.

Fix: #13983

Parent epic: #13760

Does this PR introduce any user-facing change?

No new REST API or configuration key. Creating, altering, enabling, disabling, or deleting a maintenance policy that has timeTrigger now inserts, moves, or deletes the corresponding policy-expand row while TMS scheduler pools are running.

How was this patch tested?

  • TestPolicyTimeTriggerEventListener covers create, alter, enable, disable, delete, a failed delete, and non-maintenance policies.
  • TestPolicyExpandScheduler inserts, reschedules, and removes the expand row on the H2 entity-store schema.
  • ./gradlew :plugins:tms:test --tests org.apache.gravitino.maintenance.scheduler.TestPolicyTimeTriggerEventListener --tests org.apache.gravitino.maintenance.scheduler.TestPolicyExpandScheduler --tests org.apache.gravitino.maintenance.scheduler.TestTableMaintenanceSchedulerPools -PskipITs

Made with Cursor

…cheduler

Keep the policy-expand row aligned when a maintenance policy with timeTrigger is created, altered, enabled, disabled, or deleted.

Co-authored-by: Cursor <cursoragent@cursor.com>
@lasdf1234 lasdf1234 self-assigned this Oct 16, 2026
@lasdf1234
lasdf1234 marked this pull request as draft October 16, 2026 11:49
…uler transaction

Write the policy-expand row in the same transaction as the policy change so a failed schedule rolls the policy write back.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Code Coverage Report

Overall Project 71.8% +0.02% 🟢
Files changed 78.66% 🟢

Module Coverage
aliyun 19.74% 🔴
api 54.37% 🟢
authorization-common 85.96% 🟢
authorization-ranger 14.44% 🔴
aws 53.54% 🟢
aws-bundle 0.0% 🔴
azure 32.1% 🔴
azure-bundle 0.0% 🔴
catalog-common 29.49% 🔴
catalog-fileset 82.37% 🟢
catalog-glue 73.24% 🟢
catalog-hive 83.63% 🟢
catalog-jdbc-common 57.98% 🟢
catalog-jdbc-doris 87.56% 🟢
catalog-jdbc-mysql 81.91% 🟢
catalog-jdbc-postgresql 84.02% 🟢
catalog-jdbc-starrocks 79.16% 🟢
catalog-kafka 75.21% 🟢
catalog-lakehouse-generic 84.58% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 85.93% 🟢
catalog-lakehouse-paimon 84.36% 🟢
catalog-model 77.99% 🟢
cli 44.62% 🟢
client-java 79.34% 🟢
client-java-runtime 0.0% 🔴
common 60.24% 🟢
core 87.5% +0.03% 🟢
filesystem-hadoop3 76.89% 🟢
flink 55.44% 🟢
flink-common 32.2% 🔴
flink-runtime 0.0% 🔴
gcp 32.2% 🔴
hadoop-auth 68.0% 🟢
hadoop-common 17.84% 🔴
hive-metastore-common 62.3% 🟢
hive-metastore3-libs 58.8% 🟢
iceberg-aliyun-bundle 0.0% 🔴
iceberg-common 66.53% 🟢
iceberg-rest-server 77.01% 🟢
idp-basic 86.77% 🟢
integration-test-common 0.0% 🔴
jobs 71.35% 🟢
k8s-job-executor 99.81% 🟢
lance-common 39.9% 🔴
lance-rest-server 69.51% 🟢
lineage 59.39% 🟢
optimizer 84.2% 🟢
optimizer-api 46.58% 🟢
server 91.43% 🟢
server-common 83.85% 🟢
spark 61.79% 🟢
tencent 81.78% 🟢
tms 78.23% -6.64% 🟢
trino-connector 63.31% 🟢
Files
Module File Coverage
core PolicyMetaService.java 96.64% 🟢
PolicyPreCommitHooks.java 85.71% 🟢
SqlSessions.java 82.22% 🟢
PolicyPreCommitHook.java 0.0% 🔴
tms PolicyExpandPreCommitHook.java 90.0% 🟢
TableMaintenanceSchedulerPools.java 80.0% 🟢
PolicyExpandScheduler.java 74.19% 🟢
TableMaintenanceRESTFeature.java 26.79% 🔴
PolicyExpandSchedule.java 0.0% 🔴

Co-authored-by: Cursor <cursoragent@cursor.com>
@lasdf1234 lasdf1234 changed the title [#13760] feat(tms): Register policy timeTrigger on the expand scheduler [#13983] feat(tms): Register policy timeTrigger on the expand scheduler Oct 20, 2026
@lasdf1234
lasdf1234 marked this pull request as ready for review October 21, 2026 09:51
lasdf1234 and others added 7 commits October 21, 2026 17:52
Split afterWrite/beforeDelete into persist/update/remove hooks, route
overwrite inserts through update with the retained policy id, and cover
registry plus rollback paths in tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ming

Keep the MetaService WithoutCommit convention; hooks stay inside the
method without renaming it around the extension point.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep a single PolicyExpandScheduler; the hook depends on it directly
and hook tests use the real writer against JDBC.

Co-authored-by: Cursor <cursoragent@cursor.com>
…LifecycleHook

Align the type name with the JPA-style before/after persist, update, and
remove callbacks instead of implying a single pre-commit point.

Co-authored-by: Cursor <cursoragent@cursor.com>
…scheduler

PolicyExpandLifecycleHook and PolicyExpandScheduler belong with policy
binding, not the expand/cron pool runtime under scheduler.

Co-authored-by: Cursor <cursoragent@cursor.com>
…xpandRowWriter

Name the class after writing policy-expand rows, not running the expand
pool, so it is not confused with scheduler runtime.

Co-authored-by: Cursor <cursoragent@cursor.com>
…xpandRegistrar

Name the type after registering policy-expand tasks for a timeTrigger,
not after low-level row writing.

Co-authored-by: Cursor <cursoragent@cursor.com>
@roryqi

roryqi commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

[P1] Reconcile existing policies when the scheduler starts

The new hook only runs on future policy writes. TableMaintenanceRESTFeature.configure() starts the pools and registers the hook, but it never scans existing enabled maintenance policies. On upgrade or when TMS is enabled for an existing installation, those policies have no scheduled_expand_tasks row and will never be expanded until someone edits each policy. The stopSchedulers() Javadoc also says drift is corrected on restart, but there is no restart reconciliation here. Could startup backfill/upsert these rows (and cover a pre-existing policy in a test)?

lasdf1234 and others added 5 commits October 21, 2026 19:22
Place the storage-layer PolicyLifecycleHook SPI in policy.hook so it is
not confused with PolicyHookDispatcher. Reconcile existing maintenance
policies into expand rows when TMS starts.

Co-authored-by: Cursor <cursoragent@cursor.com>
Match policy-write validation: a bad cron aborts TMS startup so the
policy must be fixed instead of being skipped with a warning.

Co-authored-by: Cursor <cursoragent@cursor.com>
…dules

Startup reconcile deletes redundant or orphan expand rows, upserts valid
due times, and clears expand rows for invalid cron instead of failing
TMS startup.

Co-authored-by: Cursor <cursoragent@cursor.com>
Document that reconcile only touches policy-expand tasks: fill missing,
reschedule wrong dues, and delete surplus expand rows. Add a stale-due
correction test.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep policy-expand tasks with due before now so the expand pool can
catch up after downtime instead of rescheduling to the next cron fire.

Co-authored-by: Cursor <cursoragent@cursor.com>
…iler UT

db-scheduler JDBC timestamps are microsecond precision; comparing a
nanosecond Instant.now() value fails round-trip assertEquals on CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
@lasdf1234 lasdf1234 closed this Oct 22, 2026
@lasdf1234 lasdf1234 reopened this Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants