xmg333 commented on PR #18463: URL: https://github.com/apache/dolphinscheduler/pull/18463#issuecomment-5561007291
Thanks, this is a good catch. I have changed the download path so that it no longer falls back to the legacy whole-file RPC. ### Changes * Removed `LogClientDelegate.writeLocalLegacy()` and `getWholeLogBytes()`. * Removed the API-side `LocalLogClient.getWholeLog()` path. * Removed the byte[] based `RemoteLogClient.getWholeLog()` and `LoggerService.getLogBytes()`. * Removed the unused `LogUtils.getFileContentBytesFromRemote()`. There is now no download path that invokes `getTaskInstanceWholeLogFileBytes`. When the chunk RPC is not available, the first-chunk failure now falls back directly to `RemoteLogClient.streamWholeLog()`, which streams the archived log in bounded chunks. If remote log storage is also unavailable, the download fails instead of trying the legacy RPC again. The error message distinguishes between: * the worker not supporting the chunk RPC, in which case it reports that the worker needs to be upgraded; * the worker being unreachable; * the worker returning a normal non-success response. The remote-storage error is preserved as the cause. I also added `RollingUpgradeLogStreamingIntegrationTest` to cover this case. The test uses an embedded Netty RPC server to simulate an old worker. The chunk RPC fails, while the legacy whole-file RPC is still available and returns a 9 MB payload. The legacy RPC invocation count is tracked with an `AtomicInteger`. The test verifies two cases: 1. Remote storage succeeds: the download is served from remote storage and the legacy whole-file RPC is never called. 2. Remote storage also fails: the download returns the worker-upgrade error and the legacy whole-file RPC is still never called. So the old worker is no longer asked to construct the whole log payload during the rolling-upgrade download path. I also removed the small-log compatibility fallback. There is currently no way to determine the log size safely before invoking the legacy RPC, so keeping that fallback would still leave the sender-side allocation unbounded. Log viewing is unaffected because it continues to use the line-bounded API. The changes are covered by the new integration test and the `LogClientDelegateTest` cases for first-chunk failures, unsupported RPCs, worker connectivity failures, remote-storage failures, empty logs, and mid-stream errors. -- 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]
