You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "deepakpanda93 (via GitHub)" <gi...@apache.org> on 2026/08/03 14:48:02 UTC
[PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
deepakpanda93 opened a new pull request, #19494:
URL: https://github.com/apache/hudi/pull/19494
### Describe the issue this Pull Request addresses
Closes #17374
Tasks writing to the same partition concurrently race to create `.hoodie_partition_metadata`. The
existence check in `HoodiePartitionMetadata.trySave()` narrows the window but cannot close it, so
the losers get an "already exists" error back from the storage layer. That error was handed to
`RetryHelper`, which logged a full stack trace at WARN and slept a second before retrying, only to
find the very file it had just been told about:
```
WARN RetryHelper: Catch Exception for N/A, will retry after 1000 ms.
org.apache.hudi.exception.HoodieIOException: Failed to create file
.../.hoodie/metadata/column_stats/.hoodie_partition_metadata
at org.apache.hudi.storage.HoodieStorage.createImmutableFileInPath(HoodieStorage.java:353)
... 40 frames ...
Caused by: org.apache.hadoop.fs.FileAlreadyExistsException: File already exists
```
The reporter saw it twice in one session, during `INSERT` and again during `CREATE INDEX`.
### Summary and Changelog
**`HoodiePartitionMetadata.trySave()` treats a lost race as the success it is.** The metafile is
immutable, so if a peer created it first, the file the caller asked for is in place. The write is
now wrapped so that on failure it re-checks for the metafile and only propagates the error if it is
genuinely absent. This removes the warning and a pointless one second stall per racing task.
The catch is a strict superset of what could previously produce the warning, which is what makes it
storage independent: `trySave` only ever retried `HoodieIOException`, `createImmutableFileInPath`
wraps every `IOException` into one, and the new catch is `IOException | HoodieIOException`. It does
not pattern match on the exception, it re-checks the filesystem, so whatever a given object store
throws for an existing key is handled.
**`RetryHelper` no longer prints a stack trace for every failed attempt.** A retry that goes on to
succeed does not warrant one, and printing it per attempt buries the genuine failures. The WARN now
carries a single line naming the failure and its root cause; the full trace is still logged at DEBUG,
and still at ERROR once the retries are exhausted.
These are separable, and reviewers may prefer them apart. The first alone closes the reported
symptom, verified below. The second is the reporter's literal suggestion, *"maybe we should avoid
logging stack traces in such warning messages"*, generalised to the other callers. Happy to split
the `RetryHelper` change into its own PR if that is preferred, since it touches twelve call sites
(lock manager, timeline client, Kafka consumer, Flink jobs) while the first change touches one
method.
### Impact
Concurrent writers no longer log an alarming stack trace, nor pay a retry backoff, for a benign
race. On a table with many partitions that is a real latency saving as well as quieter logs.
This surfaces on storage that creates the metafile directly. Local and HDFS storage write to a temp
file and rename, and a losing rename is already silent, so the noise is confined to object stores.
Note that means the reporter's original `file:///tmp` scenario no longer reproduces on master, since
`b6957f020d00` gave local storage the temp-file path.
No configuration or public API change.
### Risk Level
low
### Documentation Update
none
### Verification
| suite | result |
| --- | --- |
| `hudi-common`, `common.util.**` + `common.model.**` | 424 pass |
| `hudi-hadoop-common`, `common.model.**` + `storage.**` | 36 pass |
| `checkstyle:check`, both modules | clean |
Three tests were added, each confirmed to fail against the unmodified code:
- `TestHoodiePartitionMetadata.testTrySaveWhenMetafileConcurrentlyCreated` captures the log events
emitted while the race is lost and asserts none is WARN or worse. Without the fix it fails with
the warning it is meant to prevent: `Task [N/A] failed. current retry number 1, will retry after
1000 ms.` It also asserts the call does not pay the retry backoff.
- `TestHoodiePartitionMetadata.testConcurrentTrySaveWritesOneMetafile` runs eight writers at the
same partition and asserts one metafile and no failures.
- `TestRetryHelper.testRetryWarningCarriesNoStackTrace` drives a retry that eventually succeeds and
asserts the WARN event carries no throwable, so the logger has no trace to print, while the
message still names the failure and its root cause.
The object-store path is reached in tests by stubbing the storage scheme, not by writing to a real
bucket, so the behaviour against a live object store is argued from the exception-handling above
rather than observed.
### Contributor's checklist
- [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if 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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3712694250
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodiePartitionMetadata.java:
##########
@@ -106,22 +106,37 @@ public void trySave() throws HoodieIOException {
RetryHelper<Void, HoodieIOException> retryHelper = new RetryHelper(1000, 3, 1000, HoodieIOException.class.getName())
.tryWith(() -> {
if (!storage.exists(metaPath)) {
- if (format.isPresent()) {
- writeMetafileInFormat(metaPath, format.get());
- } else {
- // Backwards compatible properties file format
- try (ByteArrayOutputStream os = new ByteArrayOutputStream()) {
- props.store(os, "partition metadata");
- Option<byte []> content = Option.of(os.toByteArray());
- storage.createImmutableFileInPath(metaPath, content.map(HoodieInstantWriter::convertByteArrayToWriter));
+ try {
+ writeMetafile(metaPath);
+ } catch (IOException | HoodieIOException e) {
+ // The metafile is immutable, so tasks writing to the same partition concurrently race
+ // to create it and all but the winner fail with an 'already exists' error. Losing that
+ // race means the metafile is in place, which is what the caller asked for: retrying
+ // would only re-observe the same file, and warning about it is pure noise.
+ if (!storage.exists(metaPath)) {
Review Comment:
🤖 This re-check `storage.exists(metaPath)` can itself throw `IOException` (e.g. transient throttling on an object store — plausible right when the write just failed transiently). If it throws, the original `e` is lost and a raw `IOException` propagates instead. That new exception isn't in this RetryHelper's retry list (only `HoodieIOException` is), so the original retryable write failure no longer gets retried, and a checked `IOException` can slip past callers that catch only `HoodieIOException`. Would it be safer to rethrow the original `e` when the existence re-check fails, so a genuine transient write error still flows through the normal retry path?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-common/src/test/java/org/apache/hudi/common/util/TestRetryHelper.java:
##########
@@ -52,6 +61,55 @@ public void testCheckIfExceptionInRetryList() throws Exception {
assertTrue(retry);
}
+ /**
+ * The point of HUDI-9095: a retry that is going to be attempted again must not dump a stack trace
+ * into the log. The cause still has to be identifiable from the message itself.
+ */
+ @Test
+ public void testRetryWarningCarriesNoStackTrace() {
+ HoodieTestLogAppender appender = new HoodieTestLogAppender().attachTo(RetryHelper.class);
+ try {
+ AtomicInteger attempts = new AtomicInteger(0);
+ RetryHelper retryHelper = new RetryHelper(INTERVAL_TIME, 3, INTERVAL_TIME, (String) null, "save partition metafile");
+ assertDoesNotThrow(() -> retryHelper.start(() -> {
+ if (attempts.incrementAndGet() < 3) {
+ throw new IOException("Failed to create file /a/b/.hoodie_partition_metadata",
+ new FileAlreadyExistsException("File already exists: /a/b/.hoodie_partition_metadata"));
+ }
+ return true;
+ }));
+
+ List<LogEvent> warnings = appender.getLog().stream()
+ .filter(event -> Level.WARN.equals(event.getLevel())).collect(Collectors.toList());
+ assertFalse(warnings.isEmpty(), "the retries should still be reported at warn level");
+ for (LogEvent warning : warnings) {
+ assertNull(warning.getThrown(),
+ "the retry warning must not carry a throwable, otherwise the logger prints its stack trace");
+ String message = warning.getMessage().getFormattedMessage();
+ assertTrue(message.contains("save partition metafile"), message);
+ assertTrue(message.contains("java.io.IOException: Failed to create file /a/b/.hoodie_partition_metadata"), message);
+ assertTrue(message.contains("FileAlreadyExistsException"), "the root cause must survive: " + message);
+ }
+ } finally {
+ appender.detach();
+ }
+ }
+
+ @Test
Review Comment:
🤖 nit: could you rename this to `testSummarizeKeepsRootCauseOnASingleLine` to match the spelling of the production method `summarize`? The British-vs-American mismatch is a small speed-bump when scanning for which test covers which method.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5394401802
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
}, {
"hash" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16308",
"triggerID" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"triggerType" : "PUSH"
}, {
"hash" : "5dd2f5b3a53a90eee66f0c321565e7901d6aa9d4",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16504",
"triggerID" : "5dd2f5b3a53a90eee66f0c321565e7901d6aa9d4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 65f99a982c0dee84097d0a6f1cbe5d52ed578a5b Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16308)
* 5dd2f5b3a53a90eee66f0c321565e7901d6aa9d4 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16504)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "voonhous (via GitHub) via commits" <co...@hudi.apache.org>.
voonhous commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-6701227773
Temporarily closing this PR while we debug a GitHub settings error on master (#20471); it will be reopened shortly, no action needed.
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5180703407
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d197558318ceb23a785d5861cbb6997465b6d1e1 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001)
* e719f8f2df20084d2803d2f677adb1b4c45d1d22 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050)
* 2c12664e9f0aa9da641d179413fb40311b38f723 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5167961924
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d197558318ceb23a785d5861cbb6997465b6d1e1 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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3706091721
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/model/TestHoodiePartitionMetadata.java:
##########
@@ -89,6 +107,116 @@ public void testTextFormatMetaFile(Option<HoodieFileFormat> format) throws IOExc
assertEquals(3, readMetadata.getPartitionDepth());
Review Comment:
🤖 nit: `TestLogAppender` is defined identically in both `TestRetryHelper` and `TestHoodiePartitionMetadata` — could it live in a shared test-utility class (e.g. `HoodieTestLogAppender` under a `testutils` module) so the two test files can just import it?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-common/src/main/java/org/apache/hudi/common/util/RetryHelper.java:
##########
@@ -122,6 +127,20 @@ public T start() throws R {
return start(this.func);
}
+ /**
+ * Renders an exception as a single line, keeping the root cause so that the warning stays
+ * actionable without the whole stack trace.
+ */
+ @VisibleForTesting
+ static String summarise(Throwable t) {
+ Throwable rootCause = t;
Review Comment:
🤖 nit: `summarise` uses British spelling — could you rename it to `summarize` to stay consistent with Java's own conventions and the rest of the Hudi codebase (e.g. `Collectors.summarizingInt`, `DoubleSummaryStatistics`)?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5179339520
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d197558318ceb23a785d5861cbb6997465b6d1e1 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001)
* e719f8f2df20084d2803d2f677adb1b4c45d1d22 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "deepakpanda93 (via GitHub)" <gi...@apache.org>.
deepakpanda93 commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3712535714
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/model/TestHoodiePartitionMetadata.java:
##########
@@ -89,6 +107,116 @@ public void testTextFormatMetaFile(Option<HoodieFileFormat> format) throws IOExc
assertEquals(3, readMetadata.getPartitionDepth());
Review Comment:
Agreed, and done — `diff` confirmed the two copies were byte identical, which made it worth doing properly rather than leaving for the next log assertion test to copy a third time.
It now lives at `hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestLogAppender.java`. That module works for both call sites because `hudi-hadoop-common` already declares `hudi-common` with `<type>test-jar</type>`, and `org.apache.hudi.common.testutils` is where the existing shared harnesses sit.
Two things worth noting on the way it landed:
- The name matters. Surefire collects `Test*.java`, so a standalone `TestLogAppender.java` would have been picked up as a test class. `HoodieTestLogAppender` avoids that, and the class javadoc says why so nobody renames it back.
- I also moved the attach and detach boilerplate into the utility, since both call sites were repeating the same four lines of `LogManager.getLogger` / `start` / `addAppender` and the matching teardown. Each site is now `new HoodieTestLogAppender().attachTo(RetryHelper.class)` with `appender.detach()` in the `finally`.
This does mean the PR touches two modules and adds a test utility class, which is a little wider than the bug fix itself. Happy to split the extraction into a follow up if a committer would rather keep this one minimal.
Verified after the change: `TestRetryHelper` 4/4, `TestHoodiePartitionMetadata` 7/7, checkstyle and rat clean on both modules. Also rebased onto master.
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5394341549
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
}, {
"hash" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16308",
"triggerID" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"triggerType" : "PUSH"
}, {
"hash" : "5dd2f5b3a53a90eee66f0c321565e7901d6aa9d4",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5dd2f5b3a53a90eee66f0c321565e7901d6aa9d4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 65f99a982c0dee84097d0a6f1cbe5d52ed578a5b Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16308)
* 5dd2f5b3a53a90eee66f0c321565e7901d6aa9d4 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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "deepakpanda93 (via GitHub)" <gi...@apache.org>.
deepakpanda93 commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3842739461
##########
hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestLogAppender.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
+/**
+ * Collects the log events a logger emits, so a test can assert on what was logged.
+ * <p>
+ * Attach it around the code under test and detach it afterwards:
+ * <pre>{@code
+ * HoodieTestLogAppender appender = new HoodieTestLogAppender();
+ * appender.attachTo(SomeClass.class);
+ * try {
+ * // exercise the code
+ * } finally {
+ * appender.detach();
+ * }
+ * }</pre>
+ * Named so that surefire does not mistake it for a test class.
+ */
+public class HoodieTestLogAppender extends AbstractAppender {
+
+ private final List<LogEvent> log = new ArrayList<>();
Review Comment:
Agreed, renamed in `5dd2f5b3a53a`. The field is `capturedEvents` and the accessor is
`getCapturedEvents()`, including the usage example in the class javadoc.
Worth mentioning what the rename had to avoid. Grepping the repo for `getLog()` turns up five hits,
not three:
```
hudi-cli/.../TestRepairsCommand.java:310, :445
hudi-common/.../TestPriorityBasedFileSystemView.java:153, :889
hudi-common/.../HoodieTestLogAppender.java:60
```
The first four belong to two separate private `TestLogAppender` inner classes,
`TestRepairsCommand:433` and `TestPriorityBasedFileSystemView:877`, which are unrelated to this one.
A repo-wide rename would have modified two test classes this PR has no business touching, so only
this appender and its two consumers, `TestRetryHelper` and `TestHoodiePartitionMetadata`, were
changed.
That does mean the duplication raised earlier in this PR is wider than the two copies consolidated
here: there are two more appenders of essentially the same shape. Folding those into
`HoodieTestLogAppender` looks worthwhile, but this PR has already grown from a logging fix into a
test utility plus an exception handling change, so I would rather not widen it again. Happy to
follow up separately if a committer agrees.
Also rebased onto master in the same push, the branch was 26 commits behind. Re-verified afterwards:
`TestRetryHelper` 4/4, `TestHoodiePartitionMetadata` 8/8, and `checkstyle:check` plus
`apache-rat:check` clean on `hudi-common` and `hudi-hadoop-common`.
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5395545558
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
}, {
"hash" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16308",
"triggerID" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"triggerType" : "PUSH"
}, {
"hash" : "5dd2f5b3a53a90eee66f0c321565e7901d6aa9d4",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16504",
"triggerID" : "5dd2f5b3a53a90eee66f0c321565e7901d6aa9d4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5dd2f5b3a53a90eee66f0c321565e7901d6aa9d4 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16504)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5293784233
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
}, {
"hash" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16308",
"triggerID" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2c12664e9f0aa9da641d179413fb40311b38f723 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054)
* 65f99a982c0dee84097d0a6f1cbe5d52ed578a5b Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16308)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5168041596
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d197558318ceb23a785d5861cbb6997465b6d1e1 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "deepakpanda93 (via GitHub)" <gi...@apache.org>.
deepakpanda93 commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3713490213
##########
hudi-common/src/main/java/org/apache/hudi/common/model/HoodiePartitionMetadata.java:
##########
@@ -106,22 +106,37 @@ public void trySave() throws HoodieIOException {
RetryHelper<Void, HoodieIOException> retryHelper = new RetryHelper(1000, 3, 1000, HoodieIOException.class.getName())
.tryWith(() -> {
if (!storage.exists(metaPath)) {
- if (format.isPresent()) {
- writeMetafileInFormat(metaPath, format.get());
- } else {
- // Backwards compatible properties file format
- try (ByteArrayOutputStream os = new ByteArrayOutputStream()) {
- props.store(os, "partition metadata");
- Option<byte []> content = Option.of(os.toByteArray());
- storage.createImmutableFileInPath(metaPath, content.map(HoodieInstantWriter::convertByteArrayToWriter));
+ try {
+ writeMetafile(metaPath);
+ } catch (IOException | HoodieIOException e) {
+ // The metafile is immutable, so tasks writing to the same partition concurrently race
+ // to create it and all but the winner fail with an 'already exists' error. Losing that
+ // race means the metafile is in place, which is what the caller asked for: retrying
+ // would only re-observe the same file, and warning about it is pure noise.
+ if (!storage.exists(metaPath)) {
Review Comment:
Good catch, this is a real defect in what the PR adds — fixed in `2c12664e9f0a`.
Chasing it down turned the concern from "could throw" into something concrete. `RetryHelper` here is built with `HoodieIOException.class.getName()`, so `retryExceptionsClasses` is exactly `[HoodieIOException]`, and `start()` does:
```java
} catch (Exception e) {
if (!checkIfExceptionInRetryList(e)) {
throw e; // straight out, no retry
}
```
The write failure out of `createImmutableFileInPath` is a `HoodieIOException`, so it is retryable. A bare `IOException` from the existence check is not. So a single failed check did not merely lose the original: it downgraded a retryable write failure into one thrown immediately, which on an object store is precisely when the retry was worth having.
The check now cannot replace the failure it was meant to soften:
```java
private boolean metafileExistsAfterFailedWrite(StoragePath metaPath, Exception writeFailure) {
try {
return storage.exists(metaPath);
} catch (IOException checkFailure) {
writeFailure.addSuppressed(checkFailure);
return false;
}
}
```
A check that cannot answer reports the metafile as absent, so the original failure is thrown with its retry classification intact, and the check failure is kept as suppressed rather than dropped.
Covered by `testTrySaveKeepsWriteFailureWhenExistenceRecheckFails`, which stubs `exists` to slip through the pre-write check and then fail the recheck, and asserts the thrown exception is not the check failure and that the check failure survives as suppressed. Verified it is bound to the fix: reverting to the direct `storage.exists(metaPath)` fails exactly that test with `expected: not same but was: <java.io.IOException: storage unavailable>`, and leaves the other seven passing.
Worth noting for anyone reading the test: the stub alternates rather than using a fixed sequence, because the write failure is retryable. With a `doReturn(false).doThrow(...)` sequence, Mockito repeats the last stub once exhausted, so the second attempt throws from the *pre-write* check, which sits outside the try and escapes for a different reason entirely.
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5179276751
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d197558318ceb23a785d5861cbb6997465b6d1e1 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001)
* e719f8f2df20084d2803d2f677adb1b4c45d1d22 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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "deepakpanda93 (via GitHub)" <gi...@apache.org>.
deepakpanda93 commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3712533141
##########
hudi-common/src/main/java/org/apache/hudi/common/util/RetryHelper.java:
##########
@@ -122,6 +127,20 @@ public T start() throws R {
return start(this.func);
}
+ /**
+ * Renders an exception as a single line, keeping the root cause so that the warning stays
+ * actionable without the whole stack trace.
+ */
+ @VisibleForTesting
+ static String summarise(Throwable t) {
+ Throwable rootCause = t;
Review Comment:
Fair point, renamed to `summarize` in both the helper and its test.
Checked before changing it rather than assuming: the codebase had one file using the American spelling against the two I had just added with the British one, and the JDK itself is `Collectors.summarizingInt` / `DoubleSummaryStatistics`. No occurrences of `summarise` remain.
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5181470258
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e719f8f2df20084d2803d2f677adb1b4c45d1d22 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050)
* 2c12664e9f0aa9da641d179413fb40311b38f723 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5181837901
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2c12664e9f0aa9da641d179413fb40311b38f723 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "voonhous (via GitHub) via commits" <co...@hudi.apache.org>.
voonhous closed pull request #19494: fix(common): stop warning with a stack trace when a partition metafile already exists
URL: https://github.com/apache/hudi/pull/19494
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "deepakpanda93 (via GitHub)" <gi...@apache.org>.
deepakpanda93 commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3783849766
##########
hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestLogAppender.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
+/**
+ * Collects the log events a logger emits, so a test can assert on what was logged.
+ * <p>
+ * Attach it around the code under test and detach it afterwards:
+ * <pre>{@code
+ * HoodieTestLogAppender appender = new HoodieTestLogAppender();
+ * appender.attachTo(SomeClass.class);
+ * try {
+ * // exercise the code
+ * } finally {
+ * appender.detach();
+ * }
+ * }</pre>
+ * Named so that surefire does not mistake it for a test class.
+ */
+public class HoodieTestLogAppender extends AbstractAppender {
+
+ private final List<LogEvent> log = new ArrayList<>();
+ private Logger attachedLogger;
+
+ public HoodieTestLogAppender() {
+ super(UUID.randomUUID().toString(), null, null, false, null);
+ }
+
+ @Override
+ public void append(LogEvent event) {
+ // the event is mutable and gets reused, so keep an immutable copy
+ log.add(event.toImmutable());
+ }
+
+ public List<LogEvent> getLog() {
+ return new ArrayList<>(log);
+ }
+
+ /**
+ * Starts this appender and attaches it to the logger of the given class.
+ */
+ public HoodieTestLogAppender attachTo(Class<?> clazz) {
Review Comment:
Done in `65f99a982c0d`. `LogManager` is imported now and the line reads:
```java
attachedLogger = (Logger) LogManager.getLogger(clazz);
```
Worth noting why it was qualified in the first place: this class already imports
`org.apache.logging.log4j.core.Logger`, and I wanted to be sure adding
`org.apache.logging.log4j.LogManager` did not collide with it. The simple names differ, so there is
no clash and the import is clean.
Also rebased onto master in the same push. The branch had drifted 37 commits behind; GitHub still
reported it mergeable, but CI results against a base that stale are not worth much. The rebase was
clean, no conflicts.
Re-verified after both changes: `TestRetryHelper` 4/4, `TestHoodiePartitionMetadata` 8/8, and
`checkstyle:check` plus `apache-rat:check` clean on `hudi-common` and `hudi-hadoop-common`.
Checkstyle was the one to watch here, since a new import has to land in the right group for the
`ImportOrder` rule.
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5294972783
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
}, {
"hash" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16308",
"triggerID" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 65f99a982c0dee84097d0a6f1cbe5d52ed578a5b Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16308)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3786297463
##########
hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestLogAppender.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
+/**
+ * Collects the log events a logger emits, so a test can assert on what was logged.
+ * <p>
+ * Attach it around the code under test and detach it afterwards:
+ * <pre>{@code
+ * HoodieTestLogAppender appender = new HoodieTestLogAppender();
+ * appender.attachTo(SomeClass.class);
+ * try {
+ * // exercise the code
+ * } finally {
+ * appender.detach();
+ * }
+ * }</pre>
+ * Named so that surefire does not mistake it for a test class.
+ */
+public class HoodieTestLogAppender extends AbstractAppender {
+
+ private final List<LogEvent> log = new ArrayList<>();
Review Comment:
🤖 nit: could you rename `log` to `capturedEvents` (or just `events`)? In a class that is itself a log appender, `log` reads like a logger field at a quick glance, and `getLog()` returning a `List` is a small double-take that `getCapturedEvents()` would avoid.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5172855082
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d197558318ceb23a785d5861cbb6997465b6d1e1 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001)
<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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5293732117
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
}, {
"hash" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "65f99a982c0dee84097d0a6f1cbe5d52ed578a5b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2c12664e9f0aa9da641d179413fb40311b38f723 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16054)
* 65f99a982c0dee84097d0a6f1cbe5d52ed578a5b 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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5170352770
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/19494?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `65.00000%` with `7 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 76.96%. Comparing base ([`55c7a30`](https://app.codecov.io/gh/apache/hudi/commit/55c7a308094c2330189584d9df4cf03f85edd56a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d197558`](https://app.codecov.io/gh/apache/hudi/commit/d197558318ceb23a785d5861cbb6997465b6d1e1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/19494?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...che/hudi/common/model/HoodiePartitionMetadata.java](https://app.codecov.io/gh/apache/hudi/pull/19494?src=pr&el=tree&filepath=hudi-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Fmodel%2FHoodiePartitionMetadata.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL21vZGVsL0hvb2RpZVBhcnRpdGlvbk1ldGFkYXRhLmphdmE=) | 66.66% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/hudi/pull/19494?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../java/org/apache/hudi/common/util/RetryHelper.java](https://app.codecov.io/gh/apache/hudi/pull/19494?src=pr&el=tree&filepath=hudi-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Futil%2FRetryHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvUmV0cnlIZWxwZXIuamF2YQ==) | 62.50% | [1 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/hudi/pull/19494?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19494 +/- ##
============================================
- Coverage 76.97% 76.96% -0.01%
- Complexity 33855 33857 +2
============================================
Files 2575 2575
Lines 143379 143393 +14
Branches 17574 17576 +2
============================================
+ Hits 110359 110364 +5
- Misses 24759 24765 +6
- Partials 8261 8264 +3
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `82.24% <65.00%> (-0.02%)` | :arrow_down: |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.83% <ø> (ø)` | |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.96% <ø> (ø)` | |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.10% <ø> (ø)` | |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.65% <ø> (ø)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `15.32% <ø> (ø)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `63.49% <ø> (ø)` | |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.87% <ø> (ø)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.60% <ø> (ø)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `84.23% <ø> (ø)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `64.00% <ø> (ø)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/19494/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.20% <ø> (ø)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/19494/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/19494/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.76% <65.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/hudi/pull/19494?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../java/org/apache/hudi/common/util/RetryHelper.java](https://app.codecov.io/gh/apache/hudi/pull/19494?src=pr&el=tree&filepath=hudi-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Futil%2FRetryHelper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvUmV0cnlIZWxwZXIuamF2YQ==) | `91.78% <62.50%> (-3.68%)` | :arrow_down: |
| [...che/hudi/common/model/HoodiePartitionMetadata.java](https://app.codecov.io/gh/apache/hudi/pull/19494?src=pr&el=tree&filepath=hudi-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhudi%2Fcommon%2Fmodel%2FHoodiePartitionMetadata.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL21vZGVsL0hvb2RpZVBhcnRpdGlvbk1ldGFkYXRhLmphdmE=) | `85.14% <66.66%> (-3.15%)` | :arrow_down: |
... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/19494/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3718095135
##########
hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestLogAppender.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.common.testutils;
+
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
+/**
+ * Collects the log events a logger emits, so a test can assert on what was logged.
+ * <p>
+ * Attach it around the code under test and detach it afterwards:
+ * <pre>{@code
+ * HoodieTestLogAppender appender = new HoodieTestLogAppender();
+ * appender.attachTo(SomeClass.class);
+ * try {
+ * // exercise the code
+ * } finally {
+ * appender.detach();
+ * }
+ * }</pre>
+ * Named so that surefire does not mistake it for a test class.
+ */
+public class HoodieTestLogAppender extends AbstractAppender {
+
+ private final List<LogEvent> log = new ArrayList<>();
+ private Logger attachedLogger;
+
+ public HoodieTestLogAppender() {
+ super(UUID.randomUUID().toString(), null, null, false, null);
+ }
+
+ @Override
+ public void append(LogEvent event) {
+ // the event is mutable and gets reused, so keep an immutable copy
+ log.add(event.toImmutable());
+ }
+
+ public List<LogEvent> getLog() {
+ return new ArrayList<>(log);
+ }
+
+ /**
+ * Starts this appender and attaches it to the logger of the given class.
+ */
+ public HoodieTestLogAppender attachTo(Class<?> clazz) {
Review Comment:
🤖 nit: could you import `org.apache.logging.log4j.LogManager` and use `LogManager.getLogger(clazz)` here instead of the fully-qualified reference? reads a bit cleaner.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "deepakpanda93 (via GitHub)" <gi...@apache.org>.
deepakpanda93 commented on code in PR #19494:
URL: https://github.com/apache/hudi/pull/19494#discussion_r3713491951
##########
hudi-common/src/test/java/org/apache/hudi/common/util/TestRetryHelper.java:
##########
@@ -52,6 +61,55 @@ public void testCheckIfExceptionInRetryList() throws Exception {
assertTrue(retry);
}
+ /**
+ * The point of HUDI-9095: a retry that is going to be attempted again must not dump a stack trace
+ * into the log. The cause still has to be identifiable from the message itself.
+ */
+ @Test
+ public void testRetryWarningCarriesNoStackTrace() {
+ HoodieTestLogAppender appender = new HoodieTestLogAppender().attachTo(RetryHelper.class);
+ try {
+ AtomicInteger attempts = new AtomicInteger(0);
+ RetryHelper retryHelper = new RetryHelper(INTERVAL_TIME, 3, INTERVAL_TIME, (String) null, "save partition metafile");
+ assertDoesNotThrow(() -> retryHelper.start(() -> {
+ if (attempts.incrementAndGet() < 3) {
+ throw new IOException("Failed to create file /a/b/.hoodie_partition_metadata",
+ new FileAlreadyExistsException("File already exists: /a/b/.hoodie_partition_metadata"));
+ }
+ return true;
+ }));
+
+ List<LogEvent> warnings = appender.getLog().stream()
+ .filter(event -> Level.WARN.equals(event.getLevel())).collect(Collectors.toList());
+ assertFalse(warnings.isEmpty(), "the retries should still be reported at warn level");
+ for (LogEvent warning : warnings) {
+ assertNull(warning.getThrown(),
+ "the retry warning must not carry a throwable, otherwise the logger prints its stack trace");
+ String message = warning.getMessage().getFormattedMessage();
+ assertTrue(message.contains("save partition metafile"), message);
+ assertTrue(message.contains("java.io.IOException: Failed to create file /a/b/.hoodie_partition_metadata"), message);
+ assertTrue(message.contains("FileAlreadyExistsException"), "the root cause must survive: " + message);
+ }
+ } finally {
+ appender.detach();
+ }
+ }
+
+ @Test
Review Comment:
Renamed to `testSummarizeKeepsRootCauseOnASingleLine`.
Correcting my earlier reply on the other thread: I said no occurrences of `summarise` remained, and that was wrong. My rename used `sed s/summarise/summarize/g` and I verified it with `grep -n summarise` — both case sensitive, so `testSummariseKeepsRootCauseOnASingleLine` was never touched and my check could not have caught it. A case sensitive check cannot validate a case sensitive fix.
Redone with the capitalised form included and verified with `grep -rni summaris` across `.java`, `.scala` and `.md`. That now returns nothing.
--
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
Re: [PR] fix(common): stop warning with a stack trace when a partition metafile already exists [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #19494:
URL: https://github.com/apache/hudi/pull/19494#issuecomment-5180628799
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001",
"triggerID" : "d197558318ceb23a785d5861cbb6997465b6d1e1",
"triggerType" : "PUSH"
}, {
"hash" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050",
"triggerID" : "e719f8f2df20084d2803d2f677adb1b4c45d1d22",
"triggerType" : "PUSH"
}, {
"hash" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2c12664e9f0aa9da641d179413fb40311b38f723",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d197558318ceb23a785d5861cbb6997465b6d1e1 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16001)
* e719f8f2df20084d2803d2f677adb1b4c45d1d22 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=16050)
* 2c12664e9f0aa9da641d179413fb40311b38f723 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