Implement non-buffered streaming for step logs #438
Conversation
Now that the log-index is read incrementally while streaming, the length of the step log needs to be determined before returning. The only option for determining the step logs length is by reading the full index file. A new IndexReader class abstracts the parsing of the index. It is used by both the full read in advance and the incremental read while streaming the log content.
jglick
left a comment
jglick
left a comment
There was a problem hiding this comment.
Looks OK. Did not follow all of it.
| while (readNext(next)) { | ||
| stepLogSize += (next.end - next.start); | ||
| } |
There was a problem hiding this comment.
Does not seem particularly efficient, but I guess it can be optimized later as needed.
| // LargeText also needs to be improved to support opaque (non-long) cursors | ||
| // (and callers such as progressiveText.jelly and Blue Ocean updated accordingly), | ||
| // which is a hard requirement for efficient rendering of cloud-backed logs, |
There was a problem hiding this comment.
Still true I think. (Blue Ocean is moribund, but the same comment potentially applies to anything rendering JEP-210-based logs.) OTOH there has been no recent development in this area; CloudBees does maintain a cloud-backed log storage, but largely using private API contracts.
dwnusbaum
left a comment
dwnusbaum
left a comment
There was a problem hiding this comment.
Looks ok! I did not try to test it.
src/main/java/org/jenkinsci/plugins/workflow/log/FileLogStorage.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Devin Nusbaum <[email protected]>
|
jenkinsci/jenkins#11071 has been merged and included in Jenkins core |
|
Would like to test it against CloudBees CI and |
|
Actually |
|
Thanks for the reviews and merging it! Would you mind creating a new release to make it available in pipeline-graph-view-plugin? |
|
Release failed due to expired tokens: I've re-triggered it, tokens are 2 hours old so should be fine currently |
4 participants
Instead of reading the entire step log into memory before emitting parts of it, this PR is implementing streaming based reading for step logs.
Now that the log-index is read incrementally while streaming, the length
of the step log needs to be determined before returning. The only option
for determining the step logs length is by reading the full index file.
A new IndexReader class abstracts the parsing of the index. It is used
by both the full read in advance and the incremental read while
streaming the log content.
Testing done
Previous PRs have added good coverage for the happy paths and edge cases when reading the index.
Submitter checklist