Skip to content

[fix][PES][CGS] Fix broken MyBatis mapper statements in jobhistory, manager and et-monitor - #5498

Open
andrehqh wants to merge 1 commit into
apache:masterfrom
andrehqh:fix-mapper-statements
Open

andrehqh wants to merge 1 commit into
apache:masterfrom
andrehqh:fix-mapper-statements

Conversation

@andrehqh

Copy link
Copy Markdown

What is the purpose of the change

Fix the mapper statements described in #5497. They no longer match their Java mapper methods, so the calls fail at runtime:

  • jobhistory: the undone-task ID refresh (DefaultQueryCacheManager.refreshUndoneTask) fails on every run on PostgreSQL, because searchWithIdOrderAsc still uses the old parameters and maxID is missing. On MySQL it fails whenever it falls back to maxID while more than one job is in the window (TooManyResultsException).
  • manager-persistence: deleteResourceByTicketId contains a copied SELECT that uses parameters the method doesn't have.
  • et-monitor: updateIncompleteJobStatusGivenIDList iterates list instead of idList.

Only mapper XML and tests change; no Java code, configuration or DDL.

Related issues/PRs

Related issues: close #5497
Related pr: #5483 (master needs its linkis-module compile fix to build)

Brief change log

  • mapper/postgresql/JobHistoryMapper.xml: rewrite searchWithIdOrderAsc to match the method. It now uses startDate, endDate, startId and status and returns ids with LIMIT 2, like the MySQL statement, and keeps the existing ORDER BY id. Add the missing maxID.
  • mapper/mysql/JobHistoryMapper.xml: maxID selects MAX(id) instead of every id in the window.
  • ResourceManagerMapper.xml: restore deleteResourceByTicketId to delete from linkis_cg_manager_linkis_resources where ticketId = #{ticketId}, the SQL of the @Delete annotation it replaced in [ResourceManagerMapper ]The sql annotation form is optimized to XML #3618.
  • et-monitor JobHistoryMapper.xml: <foreach collection="idList">.
  • New JobHistoryMapperUndoneTaskTest runs searchWithIdOrderAsc (PostgreSQL) and maxID (MySQL and PostgreSQL) through the mapper interface on H2. It uses a plain MyBatis session because the Spring DAO tests only load the MySQL mappers.
  • ResourceManagerMapperTest#deleteResourceByTicketId also checks that resources of other tickets are kept.
  • No test for et-monitor: the module has no database test setup, and the method has no callers.

Tests

The CI builds skip tests, so I ran them locally on JDK 8 against master plus the compile fix from #5483:

Test master this PR
JobHistoryMapperUndoneTaskTest (3 cases) 3 errors: Parameter 'id' not found, TooManyResultsException, Invalid bound statement (not found): ...maxID pass
ResourceManagerMapperTest (15) deleteResourceByTicketId: Parameter 'resourceType' not found pass
all other linkis-jobhistory tests (28) pass pass
mvn -pl linkis-public-enhancements/linkis-jobhistory -am -Dtest=JobHistoryMapperUndoneTaskTest -Dsurefire.failIfNoSpecifiedTests=false test

A few notes on running these:

  • The linkis-manager-persistence pom skips its tests (<skip>true</skip>), so I ran ResourceManagerMapperTest with the skip removed locally. NodeManagerMapperTest in the same module has 10 unrelated errors both before and after this change, because its test schema has no ticketId column.
  • The Spring-based jobhistory tests need -Dorg.springframework.boot.logging.LoggingSystem=none for the test JVM, because of an unrelated Logback/Log4j2 conflict on the test classpath.

I also ran the changed statements on PostgreSQL 16 with the PostgreSQL mappers loaded. On master refreshUndoneTask() throws. With this change it sets undoneTaskMinId to the oldest unfinished job, or to the newest job id when nothing is unfinished. deleteResourceByTicketId and updateIncompleteJobStatusGivenIDList give the same results on PostgreSQL as on H2.

Build Backend, Code Format Check, Sql Script Check and the third-party dependency check currently fail on master for reasons unrelated to this change (see #5483). spotless:check reports no violations in the files changed here, and apache-rat:check passes.

Checklist

  • I have read the Contributing Guidelines on pull requests.
  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the Linkis mailing list first)
  • If this is a code change: I have written unit tests to fully verify the new behavior.

…anager and et-monitor

- jobhistory: align the PostgreSQL searchWithIdOrderAsc with the mapper method,
  add the missing PostgreSQL maxID, and select MAX(id) in maxID so it returns a
  single row
- manager: restore deleteResourceByTicketId to delete by ticketId
- et-monitor: iterate idList in updateIncompleteJobStatusGivenIDList

close apache#5497
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.

[Bug] Broken MyBatis mapper statements in jobhistory, manager-persistence and et-monitor

1 participant