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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to