xmg333 commented on PR #18463:
URL:
https://github.com/apache/dolphinscheduler/pull/18463#issuecomment-5188106742
Thanks for the review @SbloodyS . I've reworked the approach based on your
feedback. New ** chunked streaming log ** is now completed. Could you confirm
if this scope is what you had in mind?
## What's new
**New RPC:** `getTaskInstanceLogFileChunk(path, offset, length)` reads an
8 MB range via `RandomAccessFile`. The API loops this RPC and streams
each chunk to the HTTP response via `StreamingResponseBody`.
## Fallback logic (the important part)
The API server runs a chunked loop with an `offset` counter tracking bytes
already written to the response:
```
streamWholeLog(taskInstance, outputStream):
if worker not in registry:
→ remote getWholeLogBytes (legacy, unchanged)
offset = 0
loop:
try:
chunk = localLogClient.getLogChunk(offset, 8MB)
if chunk.code != SUCCESS:
if offset == 0: ← nothing written yet, safe to fallback
→ remote getWholeLogBytes; return
else: ← bytes already streamed, can't restart
→ throw IOException
write chunk.bytes; offset += chunk.bytes.length
if chunk.eof: return
catch Exception: ← old worker (method not found),
timeout, etc.
if offset == 0: ← still safe
→ remote getWholeLogBytes; return
else:
→ throw IOException
```
**The core invariant:** fallback only happens when `offset == 0` (nothing
written yet). Once bytes have been streamed (`offset > 0`), there's no
safe way to restart — falling back to `getWholeLogBytes` would write the
whole file from the beginning, duplicating the prefix that's already in
the response. So mid-stream failures throw instead.
**Three concrete scenarios:**
| Scenario | offset | Behavior |
|----------|--------|----------|
| Old worker, first chunk RPC fails (method not found) | 0 | → fallback to
legacy `getWholeLogBytes` (rolling upgrade safe) |
| Worker dies mid-stream after writing 24 MB | 24 MB | → throw
`IOException` (client sees truncated download, not corrupted) |
| Worker offline from the start | 0 | → go directly to remote
`getWholeLogBytes` |
**Legacy path unchanged:** `getTaskInstanceWholeLogFileBytes` and
`getFileContentBytesFromLocal` are untouched from upstream/dev — no silent
truncation, no size cap added.
**`readFileRange`** (the new reader) fails explicitly: missing file →
`IOException`, not empty bytes.
Tests cover all three scenarios in `LogClientDelegateTest`.
--
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]
