lasdf1234 commented on code in PR #14054:
URL: https://github.com/apache/gravitino/pull/14054#discussion_r4335287353
##########
plugins/tms/src/main/java/org/apache/gravitino/maintenance/occupancy/OccupancyGate.java:
##########
@@ -78,22 +82,62 @@ public OccupancyGate() {
/**
* @param tableId table entity id
* @param policyId policy entity id
- * @return true when any open row is a live unbound claim or an active
Gravitino job
+ * @return true when any open row is a live unbound claim or a still-bound
soft-slot row
*/
public boolean isInFlight(long tableId, long policyId) {
+ reclaimTerminalBound(tableId, policyId);
if (actionMetaService.hasLiveUnboundClaim(tableId, policyId,
claimTimeoutMs)) {
return true;
}
+ // After reclaim, any remaining bound open row occupies the soft slot
(ACTIVE job, or a
+ // terminal/missing job whose markFinished raced/failed). Fail closed so
UNIQUE(table, policy,
+ // finished_at=0) cannot be ignored while still blocking
tryAcquireInFlight.
+ for (TableMaintenanceActionPO open :
actionMetaService.listInFlight(tableId, policyId)) {
+ if (open.jobId() != null) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Job-terminal reconciler failsafe (ยง5.2.2): when Spark finish callback was
missed, mark the
+ * action finished using the best-known snapshot id. Does not invent stub
metrics rows (those
+ * would pollute {@code table_snapshot_metrics}).
+ *
+ * @param tableId table entity id
+ * @param policyId policy entity id
+ */
+ public void reclaimTerminalBound(long tableId, long policyId) {
for (TableMaintenanceActionPO open :
actionMetaService.listInFlight(tableId, policyId)) {
if (open.jobId() == null) {
continue;
}
- JobEntity job = loadJob(open.jobId(), tableId);
+ JobEntity job = loadJob(open.jobId(), policyId);
+ // Missing job meta (purged / unresolvable) must free the soft slot;
otherwise UNIQUE(table,
+ // policy, finished_at=0) blocks every future acquire.
if (job != null && ACTIVE.contains(job.status())) {
- return true;
+ continue;
+ }
+ long finishedAt =
+ job != null && job.finishedAt() != null && job.finishedAt() > 0
+ ? job.finishedAt()
+ : System.currentTimeMillis();
+ Long afterSnapshotId =
Review Comment:
Good catch โ went with the finish-path backfill.
`markFinished` now:
1. tries the normal close (`finished_at = 0`);
2. if that updates 0 rows and `afterSnapshotId` is non-null, backfills
`after_snapshot_id` on an already-closed row (`finished_at > 0 AND
after_snapshot_id IS NULL`), keeping the reclaim `finished_at`.
So reclaim stays a failsafe (still never invents S1 from S0), and a late
finish listener can still record the real S1.
Covered by `testMarkFinishedBackfillsAfterSnapshotWhenAlreadyClosed` and an
extension of `testReclaimDoesNotCopyBeforeSnapshotIdIntoAfter` (terminal โ
reclaim โ finish(S1)).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
