[I] [Bug]: Persisted task output UID mismatch leaves optimizing processes RUNNING during AMS recovery [amoro]

["j1wonpark (via GitHub)" <gi...@apache.org>]

j1wonpark opened a new issue, #4441:
URL: https://github.com/apache/amoro/issues/4441

   ### What happened?
   
   This is a follow-up to #3261 and its fix, #3262.
   
   #3262 moved `selectTaskRuntimes()` inside the recovery `try` block, but the corresponding handler only catches `IllegalArgumentException`.
   
   When persisted task output cannot be deserialized because of a `serialVersionUID` mismatch, the persistence layer wraps the exception:
   
   ```text
   org.apache.amoro.exception.PersistenceException
   └─ org.apache.ibatis.exceptions.PersistenceException
      └─ java.lang.IllegalArgumentException: deserialization error
         └─ java.io.InvalidClassException: org.apache.iceberg.BaseFile
   ```
   
   This wrapped exception bypasses the existing handler. During AMS recovery, table runtime initialization logs an error and skips the affected table. The persisted optimizing process remains `RUNNING`, and the table remains in its optimizing state without being registered with the scheduler.
   
   Expected behavior:
   
   - Close the unrecoverable process as `CLOSED`.
   - Reset the table to `IDLE` and register it with the scheduler so subsequent optimization can proceed.
   - Preserve the existing behavior for unrelated persistence failures, such as database access errors, without closing the process.
   
   ### Affects Versions
   
   master
   
   ### What table formats are you seeing the problem on?
   
   Iceberg
   
   ### What engines are you seeing the problem on?
   
   AMS
   
   ### How to reproduce
   
   The local regression test `testRecoveryClosesProcessWithIncompatibleTaskOutput` reproduces the issue through the actual database persistence and queue initialization paths:
   
   1. Create an Iceberg table and an optimizing process.
   2. Complete a task and persist its output in the database.
   3. Prepare the recovery state with the process still `RUNNING` and the table in `MINOR_OPTIMIZING`, representing recovery before the process transitions to committing.
   4. Modify the `serialVersionUID` in the serialized `org.apache.iceberg.BaseFile` class descriptor within the persisted task output so it differs from the class loaded by AMS.
   5. Reconstruct the table runtime from persisted state and initialize the optimizing queue.
   
   Without the fix:
   
   - Task output deserialization fails with a wrapped `InvalidClassException`.
   - The process remains `RUNNING`.
   - The table remains `MINOR_OPTIMIZING`.
   - The table is not registered with the scheduler.
   
   The test deliberately modifies the UID to reproduce the incompatibility deterministically. It does not perform a full Iceberg version upgrade or restart the AMS JVM.
   
   The test and proposed fix are available in [this commit](https://github.com/j1wonpark/amoro/commit/07e60b51e45fe2d681b161656655f5f308b510f8). To observe the regression test fail, apply the test without the production change to `OptimizingQueue.java`.
   
   ### Relevant log output
   
   Excerpt from the local regression test with a deliberately modified `serialVersionUID`; unrelated frames omitted:
   
   ```text
   ERROR org.apache.amoro.server.optimizing.OptimizingQueue:
   Failed to initialize table runtime for table
   test_catalog.test_database.task_recovery(tableId=4), skipping
   
   org.apache.amoro.exception.PersistenceException:
   org.apache.ibatis.exceptions.PersistenceException:
   ...
   Caused by: java.lang.IllegalArgumentException: deserialization error
   ...
   Caused by: java.io.InvalidClassException: org.apache.iceberg.BaseFile;
   local class incompatible:
   stream classdesc serialVersionUID = -4852755190289257283,
   local class serialVersionUID = -4780697596251329347
   ```
   
   ### Anything else
   
   Related issues and fixes:
   
   - #2521 / #2524 introduced closing optimizing processes when task input recovery fails.
   - #3261 / #3262 addressed task recovery after an Iceberg upgrade. The log in #3261 also shows the persistence exception wrapping described above.
   
   This issue focuses on handling incompatible persisted task output during recovery. Changing the serialization format or defining Iceberg version compatibility policies is outside its scope.
   
   The regression test fails without the fix and passes with it. An additional test verifies that an unrelated database error does not close the process.
   
   ### Are you willing to submit a PR?
   
   - [x] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [x] I agree to follow this project's Code of Conduct
   


-- 
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: commits-unsubscribe@amoro.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug]: Persisted task output UID mismatch leaves optimizing processes RUNNING during AMS recovery [amoro]

["j1wonpark (via GitHub)" <gi...@apache.org>]

j1wonpark closed issue #4441: [Bug]: Persisted task output UID mismatch leaves optimizing processes RUNNING during AMS recovery
URL: https://github.com/apache/amoro/issues/4441


-- 
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: commits-unsubscribe@amoro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org