[GitHub] [flink] XComp commented on a diff in pull request #22072: [FLINK-31092][sql-gateway][table-common] Fix Hive ITCase fail with OutOfMemoryError

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

XComp commented on code in PR #22072:
URL: https://github.com/apache/flink/pull/22072#discussion_r1123179109


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/factories/ServiceLoaderUtil.java:
##########
@@ -44,6 +51,20 @@ static <T> List<LoadResult<T>> load(Class<T> clazz, ClassLoader classLoader) {
             } catch (NoSuchElementException e) {
                 break;
             } catch (Throwable t) {
+                if (t instanceof NoClassDefFoundError) {
+                    LOG.debug(
+                            "NoClassDefFoundError when loading a "
+                                    + clazz.getCanonicalName()
+                                    + ". This is expected when trying to load a format dependency but no flink-connector-files is loaded.",
+                            t);
+                    // After logging, we just ignore this failure
+                    continue;
+                }
+
+                if (!clazz.isInstance(t)) {

Review Comment:
   imho, this if condition still doesn't make sense. And the error handling should probably live in the calling method `FactoryUtil.discoverFactories`. `ServiceLoaderUtil.load` seems to be more general. I provided a commit show what I had in mind in [42453004](https://github.com/fsk119/flink/pull/1/commits/42453004898473a6e28b78f7603b30c3b1e24951) in [PR fsk119/flink:#1](https://github.com/fsk119/flink/pull/1/). But we could even go one step further and delete `ServiceLoaderUtil` completely. imho, it doesn't add much value to the codebase as it's only called in `FactoryUtil.discoverFactories`.
   
   But see [PR fsk119/flink:#1](https://github.com/fsk119/flink/pull/1/) more like a suggestion. ...it's your call.



-- 
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: issues-unsubscribe@flink.apache.org

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