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