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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
