xmg333 commented on PR #18463: URL: https://github.com/apache/dolphinscheduler/pull/18463#issuecomment-5974259809
Thanks for the review. Both points are addressed. I also did another pass over the affected paths and fixed several related issues in the same commit. ### 1. Remote download failures now propagate, and only complete archives are published * `RemoteLogHandler#getRemoteLog` now propagates `IOException` through all five handlers. `RemoteLogUtils#getRemoteLog` logs and rethrows instead of treating a failed download as success. * All handlers download into a private staging file (`<archive>.download-<uuid>`) and publish it with an atomic move. A failed transfer removes the staging file, leaves any existing archive untouched, and fails the request. * S3/ABS verify the received byte count against the announced object size, and GCS explicitly reports a missing object. * The log-view path also fails explicitly instead of falling back to a stale or partial local file. * Leftover staging and snapshot files are now also swept in the archive's own directory before each download, using the orphan age gate. This is needed because the startup sweep only walks the local log base dir, which is not where task archives are downloaded to. Added regression tests covering mid-transfer failures and existing cached archives. They verify that a failed download never replaces or serves a partial archive. ### 2. PR description The PR description has been updated to match the current implementation: 8 MB chunked streaming, with the old 47 MB limit removed. ### Additional issues found during self-review * Live-log downloads now pin their target length to the file length the worker reports on the first chunk, so a growing log ends cleanly at that snapshot boundary instead of keeping the download open until the async timeout. * Old-worker detection now uses a typed `MethodNotFoundException` instead of parsing error messages. Invocation failures from current workers are reported separately. * The fixed 64-stripe log locks were replaced with per-log-path, reference-counted locks, so unrelated log downloads no longer block each other. * RPC wait failures now preserve their original cause. * Large chunk contents are no longer included in RPC error logs. * Replaced the ineffective regression test whose counter was incremented by its own stub with assertions against the actual streaming behavior and bounded writes. * Added the required `BREAKING CHANGE` documentation for the `RemoteLogHandler#getRemoteLog` signature change and removal of `LogUtils#getFileContentBytesFromRemote`. The changes are in the latest commit `744d282`. -- 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]
