jerryshao opened a new pull request, #12675:
URL: https://github.com/apache/gravitino/pull/12675

   ### What changes were proposed in this pull request?
   
   Replaces the `entityStore.put(entity, overwrite=true)` + manual 
re-fetch-under-lock pattern in `JobManager.cancelJob`/`pullAndUpdateJobStatus` 
with `entityStore.update(...)`, matching the pattern already used for 
`JOB_TEMPLATE` and other entities:
   
   - Adds `JobMetaService.updateJob` (plus the `JDBCBackend` dispatch case, the 
`updateJobMeta` mapper/SQL provider method, and `JobPO.updateJobPO`) needed to 
route `JOB` entity writes through `entityStore.update()`.
   - `updateJobMeta`'s `WHERE` clause now does a proper 
`current_version`/`last_version` compare-and-swap, matching the same pattern 
used by `View`/`Table`/`JobTemplate`.
   - `JobManager.cancelJob` now translates a `NoSuchEntityException` from a 
concurrently deleted job into `NoSuchJobException` (matching `getJob`'s 
existing behavior) instead of leaking an undeclared exception.
   - `JobManager.pullAndUpdateJobStatus` now catches `NoSuchEntityException` 
per-job and skips it (logging a warning) instead of letting the exception 
escape the scheduled task — previously this would have caused 
`ScheduledExecutorService` to silently and permanently cancel all future 
status-poll runs.
   - Also includes an incidental, unrelated commit reformatting two long 
`assert_properties_equal` calls in the Python integration tests (no behavior 
change).
   
   ### Why are the changes needed?
   
   The job entity was the only entity type still using a raw `put(overwrite)` 
for updates, requiring every call site to manually re-fetch the latest state 
under a lock before overwriting — a pattern that's easy to get wrong (see the 
removed comment in `cancelJob` describing a near-miss with a stale snapshot) 
and inconsistent with every other entity in `JDBCBackend`.
   
   Fix: #12669
   
   ### Does this PR introduce _any_ user-facing change?
   
   No user-facing/API change. Internal storage-layer behavior only.
   
   ### How was this patch tested?
   
   Added/updated unit tests:
   - `TestJobMetaService`: `testUpdateJob`, 
`testUpdateNonExistentJobThrowsNoSuchEntityException`, 
`testUpdateJobWithMismatchedIdThrowsIllegalArgumentException`, 
`testUpdateJobWithMalformedIdentifierThrowsNoSuchEntityException`.
   - `TestJobPO`: `testUpdateJobPO`.
   - `TestJobManager`: updated existing `cancelJob`/`pullAndUpdateJobStatus` 
tests to mock `entityStore.update()` instead of `put()`; added 
`testCancelJobThrowsNoSuchJobExceptionWhenJobDeletedConcurrently` and 
`testPullJobStatusSkipsJobDeletedConcurrently`.
   
   All of `TestJobManager`, `TestJobMetaService`, and `TestJobPO` pass locally.


-- 
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]

Reply via email to