You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "yihua (via GitHub)" <gi...@apache.org> on 2023/05/17 06:04:44 UTC
[GitHub] [hudi] yihua opened a new pull request, #8732: [HUDI-6227] Improve logging in `ReflectionUtils.hasConstructor`
yihua opened a new pull request, #8732:
URL: https://github.com/apache/hudi/pull/8732
### Change Logs
`ReflectionUtils.hasConstructor` throws warn log if the constructor does not exist, even in cases where it's expected, e.g., for `DataHubSyncTool` (see below), confusing users. This PR improves the logging so that by default the debug-level logs are used.
```
23/05/16 10:58:54 WARN ReflectionUtils: Unable to instantiate class org.apache.hudi.sync.datahub.DataHubSyncTool
java.lang.NoSuchMethodException: org.apache.hudi.sync.datahub.DataHubSyncTool.<init>(java.util.Properties, org.apache.hadoop.conf.Configuration)
at java.lang.Class.getConstructor0(Class.java:3082)
at java.lang.Class.getConstructor(Class.java:1825)
at org.apache.hudi.common.util.ReflectionUtils.hasConstructor(ReflectionUtils.java:95)
at org.apache.hudi.sync.common.util.SyncUtilHelpers.instantiateMetaSyncTool(SyncUtilHelpers.java:83)
at org.apache.hudi.sync.common.util.SyncUtilHelpers.runHoodieMetaSync(SyncUtilHelpers.java:58)
at org.apache.hudi.utilities.deltastreamer.DeltaSync.runMetaSync(DeltaSync.java:808)
at org.apache.hudi.utilities.deltastreamer.DeltaSync.writeToSink(DeltaSync.java:723)
at org.apache.hudi.utilities.deltastreamer.DeltaSync.syncOnce(DeltaSync.java:395)
at org.apache.hudi.utilities.deltastreamer.HoodieDeltaStreamer.lambda$sync$2(HoodieDeltaStreamer.java:215)
at org.apache.hudi.common.util.Option.ifPresent(Option.java:97)
at org.apache.hudi.utilities.deltastreamer.HoodieDeltaStreamer.sync(HoodieDeltaStreamer.java:213)
at org.apache.hudi.utilities.deltastreamer.HoodieDeltaStreamer.main(HoodieDeltaStreamer.java:592)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.apache.spark.deploy.JavaMainApplication.start(SparkApplication.scala:52)
at org.apache.spark.deploy.SparkSubmit.org$apache$spark$deploy$SparkSubmit$$runMain(SparkSubmit.scala:955)
at org.apache.spark.deploy.SparkSubmit.doRunMain$1(SparkSubmit.scala:180)
at org.apache.spark.deploy.SparkSubmit.submit(SparkSubmit.scala:203)
at org.apache.spark.deploy.SparkSubmit.doSubmit(SparkSubmit.scala:90)
at org.apache.spark.deploy.SparkSubmit$$anon$2.doSubmit(SparkSubmit.scala:1043)
at org.apache.spark.deploy.SparkSubmit$.main(SparkSubmit.scala:1052)
at org.apache.spark.deploy.SparkSubmit.main(SparkSubmit.scala)
```
### Impact
Improves logging to be user-friendly.
### Risk level
none
### Documentation Update
N/A
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yihua merged pull request #8732: [HUDI-6227] Improve logging in `ReflectionUtils.hasConstructor`
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua merged PR #8732:
URL: https://github.com/apache/hudi/pull/8732
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8732: [HUDI-6227] Improve logging in `ReflectionUtils.hasConstructor`
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8732:
URL: https://github.com/apache/hudi/pull/8732#issuecomment-1550857054
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "518ea5a43b1a4e6795a0e671d3e4c82d128f289c",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17144",
"triggerID" : "518ea5a43b1a4e6795a0e671d3e4c82d128f289c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 518ea5a43b1a4e6795a0e671d3e4c82d128f289c Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17144)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8732: [HUDI-6227] Improve logging in `ReflectionUtils.hasConstructor`
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8732:
URL: https://github.com/apache/hudi/pull/8732#issuecomment-1551245385
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "518ea5a43b1a4e6795a0e671d3e4c82d128f289c",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17144",
"triggerID" : "518ea5a43b1a4e6795a0e671d3e4c82d128f289c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 518ea5a43b1a4e6795a0e671d3e4c82d128f289c Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=17144)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] yihua commented on a diff in pull request #8732: [HUDI-6227] Improve logging in `ReflectionUtils.hasConstructor`
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #8732:
URL: https://github.com/apache/hudi/pull/8732#discussion_r1197105077
##########
hudi-common/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java:
##########
@@ -83,19 +83,40 @@ public static Object loadClass(String clazz, Class<?>[] constructorArgTypes, Obj
}
/**
- * Check if the clazz has the target constructor or not.
+ * Check if the clazz has the target constructor or not, without throwing warn-level log.
*
+ * @param clazz Class name.
+ * @param constructorArgTypes Argument types of the constructor.
+ * @return
+ */
+ public static boolean hasConstructor(String clazz, Class<?>[] constructorArgTypes) {
+ return hasConstructor(clazz, constructorArgTypes, true);
+ }
+
+ /**
+ * Check if the clazz has the target constructor or not.
+ * <p>
* When catch {@link HoodieException} from {@link #loadClass}, it's inconvenient to say if the exception was thrown
* due to the instantiation's own logic or missing constructor.
- *
+ * <p>
* TODO: ReflectionUtils should throw a specific exception to indicate Reflection problem.
+ *
+ * @param clazz Class name.
+ * @param constructorArgTypes Argument types of the constructor.
+ * @param silenceWarning {@code true} to use debug-level logging; otherwise, use warn-level logging.
+ * @return {@code true} if the constructor exists; {@code false} otherwise.
*/
- public static boolean hasConstructor(String clazz, Class<?>[] constructorArgTypes) {
+ public static boolean hasConstructor(String clazz, Class<?>[] constructorArgTypes, boolean silenceWarning) {
Review Comment:
Not sure if I follow you. Production code in `SyncUtilHelpers.instantiateMetaSyncTool` uses this method and throws warning logs which are noises to users (this tries all possible arg combinations until the right one is found; the wrong arg combination throws noisy warning log before this PR). This is irrelevant to tests.
```
if (ReflectionUtils.hasConstructor(syncToolClassName,
new Class<?>[] {Properties.class, Configuration.class})) {
return ((HoodieSyncTool) ReflectionUtils.loadClass(syncToolClassName,
new Class<?>[] {Properties.class, Configuration.class},
properties, hadoopConfig));
} else if (ReflectionUtils.hasConstructor(syncToolClassName,
new Class<?>[] {Properties.class})) {
return ((HoodieSyncTool) ReflectionUtils.loadClass(syncToolClassName,
new Class<?>[] {Properties.class},
properties));
} else if (ReflectionUtils.hasConstructor(syncToolClassName,
new Class<?>[] {TypedProperties.class, Configuration.class, FileSystem.class})) {
return ((HoodieSyncTool) ReflectionUtils.loadClass(syncToolClassName,
new Class<?>[] {TypedProperties.class, Configuration.class, FileSystem.class},
properties, hadoopConfig, fs));
} else if (ReflectionUtils.hasConstructor(syncToolClassName,
new Class<?>[] {Properties.class, FileSystem.class})) {
return ((HoodieSyncTool) ReflectionUtils.loadClass(syncToolClassName,
new Class<?>[] {Properties.class, FileSystem.class},
properties, fs));
} else {
throw new HoodieException("Could not load meta sync class " + syncToolClassName
+ ": no valid constructor found.");
}
```
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on a diff in pull request #8732: [HUDI-6227] Improve logging in `ReflectionUtils.hasConstructor`
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #8732:
URL: https://github.com/apache/hudi/pull/8732#discussion_r1197073022
##########
hudi-common/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java:
##########
@@ -83,19 +83,40 @@ public static Object loadClass(String clazz, Class<?>[] constructorArgTypes, Obj
}
/**
- * Check if the clazz has the target constructor or not.
+ * Check if the clazz has the target constructor or not, without throwing warn-level log.
*
+ * @param clazz Class name.
+ * @param constructorArgTypes Argument types of the constructor.
+ * @return
+ */
+ public static boolean hasConstructor(String clazz, Class<?>[] constructorArgTypes) {
+ return hasConstructor(clazz, constructorArgTypes, true);
+ }
+
+ /**
+ * Check if the clazz has the target constructor or not.
+ * <p>
* When catch {@link HoodieException} from {@link #loadClass}, it's inconvenient to say if the exception was thrown
* due to the instantiation's own logic or missing constructor.
- *
+ * <p>
* TODO: ReflectionUtils should throw a specific exception to indicate Reflection problem.
+ *
+ * @param clazz Class name.
+ * @param constructorArgTypes Argument types of the constructor.
+ * @param silenceWarning {@code true} to use debug-level logging; otherwise, use warn-level logging.
+ * @return {@code true} if the constructor exists; {@code false} otherwise.
*/
- public static boolean hasConstructor(String clazz, Class<?>[] constructorArgTypes) {
+ public static boolean hasConstructor(String clazz, Class<?>[] constructorArgTypes, boolean silenceWarning) {
Review Comment:
I am not in favor of adding additional args just for testing purpose. Tests should enable debug logs and verify
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on a diff in pull request #8732: [HUDI-6227] Improve logging in `ReflectionUtils.hasConstructor`
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #8732:
URL: https://github.com/apache/hudi/pull/8732#discussion_r1197110775
##########
hudi-common/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java:
##########
@@ -83,19 +83,40 @@ public static Object loadClass(String clazz, Class<?>[] constructorArgTypes, Obj
}
/**
- * Check if the clazz has the target constructor or not.
+ * Check if the clazz has the target constructor or not, without throwing warn-level log.
*
+ * @param clazz Class name.
+ * @param constructorArgTypes Argument types of the constructor.
+ * @return
+ */
+ public static boolean hasConstructor(String clazz, Class<?>[] constructorArgTypes) {
+ return hasConstructor(clazz, constructorArgTypes, true);
+ }
+
+ /**
+ * Check if the clazz has the target constructor or not.
+ * <p>
* When catch {@link HoodieException} from {@link #loadClass}, it's inconvenient to say if the exception was thrown
* due to the instantiation's own logic or missing constructor.
- *
+ * <p>
* TODO: ReflectionUtils should throw a specific exception to indicate Reflection problem.
+ *
+ * @param clazz Class name.
+ * @param constructorArgTypes Argument types of the constructor.
+ * @param silenceWarning {@code true} to use debug-level logging; otherwise, use warn-level logging.
+ * @return {@code true} if the constructor exists; {@code false} otherwise.
*/
- public static boolean hasConstructor(String clazz, Class<?>[] constructorArgTypes) {
+ public static boolean hasConstructor(String clazz, Class<?>[] constructorArgTypes, boolean silenceWarning) {
Review Comment:
sg then.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8732: [HUDI-6227] Improve logging in `ReflectionUtils.hasConstructor`
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8732:
URL: https://github.com/apache/hudi/pull/8732#issuecomment-1550847462
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "518ea5a43b1a4e6795a0e671d3e4c82d128f289c",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "518ea5a43b1a4e6795a0e671d3e4c82d128f289c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 518ea5a43b1a4e6795a0e671d3e4c82d128f289c UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org