[PR] [Improvement] Graceful handling of catalog initialization failures in DefaultCatalogManager [amoro]

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

j1wonpark opened a new pull request, #4189:
URL: https://github.com/apache/amoro/pull/4189

   ## Why are the changes needed?
   
   When an external catalog (e.g., REST catalog) fails to initialize during AMS startup — due to an unreachable endpoint, authentication error, or network issue — the exception propagates from `DefaultCatalogManager`'s constructor, crashing the entire AMS process. This means a single misconfigured or temporarily unavailable catalog prevents all other catalogs and the dashboard from being accessible.
   
   Furthermore, once AMS restarts with the init try-catch, the failed catalog cannot be updated via the dashboard because `updateCatalog()` calls `getServerCatalog()`, which attempts to rebuild the catalog using `computeIfAbsent` with the old (broken) metadata, triggering the same failure.
   
   ## Brief change log
   
   - `DefaultCatalogManager`: Wrap individual catalog loading in try-catch during initialization so that a single catalog failure is logged and skipped rather than crashing AMS
   - `DefaultCatalogManager.updateCatalog()`: When a catalog is not present in `serverCatalogMap` (failed to initialize), fall back to updating DB directly and attempt to rebuild with the new metadata, allowing operators to fix catalog configuration from the dashboard without restarting AMS
   - `DefaultCatalogManager.removeServerCatalog()`: Add `@VisibleForTesting` helper method for test support
   - `TestDefaultCatalogManagerRecovery`: Add tests verifying both initialization resilience and update recovery for failed catalogs
   
   ## How was this patch tested?
   
   - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
     - `testInitializationSkipsFailedCatalog`: Verifies that `DefaultCatalogManager` constructor does not throw when a REST catalog with an unreachable endpoint exists in the database
     - `testUpdateCatalogThatFailedToInitialize`: Verifies that `updateCatalog()` succeeds for a catalog that exists in DB but failed to load into `serverCatalogMap`
     - Both tests verified to FAIL without the fix and PASS with the fix
   
   - [x] Run test locally before making a pull request
   
   ## Documentation
   
   - Does this pull request introduce a new feature? No
   - If yes, how is the feature documented? Not applicable


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

Re: [PR] [AMORO-4190][ams] Graceful handling of catalog initialization failures in DefaultCatalogManager [amoro]

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

czy006 merged PR #4189:
URL: https://github.com/apache/amoro/pull/4189


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