[HUDI-4950] Fix read log lead to oom not be catched issue#6827
[HUDI-4950] Fix read log lead to oom not be catched issue#6827TJX2014 wants to merge 1 commit intoapache:masterfrom
Conversation
| return getTableAvroSchema(metaClient, false); | ||
| } catch (Exception e) { | ||
| LOG.warn("Error while resolving the latest table schema", e); | ||
| } catch (Throwable throwable) { | ||
| LOG.warn("Error while resolving the latest table schema.", throwable); | ||
| // ignored | ||
| } |
There was a problem hiding this comment.
it's not a good practice to ignore Throwable. why don't you want to fail loud here?
There was a problem hiding this comment.
Just to consistent with org.apache.hudi.table.catalog.HoodieCatalog#getLatestTableSchema which catch Throwable rather than Exception, why only ignore Exception, there are many situation that not the subclass of Exception for example oom.
There was a problem hiding this comment.
@TJX2014 then getLatestTableSchema should be fixed too. We don't catch Throwable because Error should not be caught. Quote from javadoc
An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch
So what is the strong reason to catch and ignore errors like OOM? You'd need to fail loud in that case.
nsivabalan
added
priority:critical
|
as mentioned in this comment https://github.com/apache/hudi/pull/6827/files#r985062923 we should make some improvements for places where catch Throwable. I'm closing this now. If further discussion needed, we can continue it here |
4 participants
Change Logs
Make org.apache.hudi.util.StreamerUtil#getLatestTableSchema can catch oom exception caused by read log/base file to infer schema
Impact
Use HoodieHiveCatalog to infer schema can be consistent with HoodieCatalog, which can catch oom.
Risk level: none | low | medium | high
none
Documentation Update
none
Contributor's checklist