leocook commented on code in PR #18210:
URL:
https://github.com/apache/dolphinscheduler/pull/18210#discussion_r3178285873
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-remoteshell/src/main/java/org/apache/dolphinscheduler/plugin/task/remoteshell/RemoteExecutor.java:
##########
@@ -235,24 +235,28 @@ public String runRemote(String command) throws
IOException {
private int runRemoteAndProcessLines(String command, Consumer<String>
lineConsumer) throws IOException {
try (
ChannelExec channel = getSession().createExecChannel(command);
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
ByteArrayOutputStream err = new ByteArrayOutputStream()) {
- InputStream out = channel.getInvertedOut();
+ channel.setOut(out);
channel.setErr(err);
channel.open();
+ channel.waitFor(EnumSet.of(ClientChannelEvent.CLOSED), 0);
+ Integer exitStatus = channel.getExitStatus();
+ if (exitStatus == null || exitStatus != 0) {
+ throw new TaskException(
+ "Remote shell task error, exitStatus: " + exitStatus +
" error message: " + err);
+ }
int readLines = 0;
- try (BufferedReader reader = new BufferedReader(new
InputStreamReader(out, StandardCharsets.UTF_8))) {
+ try (
+ BufferedReader reader = new BufferedReader(
+ new InputStreamReader(new
ByteArrayInputStream(out.toByteArray()),
+ StandardCharsets.UTF_8))) {
Review Comment:
Acknowledged. A custom line-emitting OutputStream passed to channel.setOut()
could achieve true O(line) memory without getInvertedOut(). However, it
introduces I/O-thread callback concerns and multi-byte UTF-8 boundary handling
complexity. Since RemoteShell output is typically KB-scale log lines, the
current ByteArrayOutputStream + toByteArray() approach is an acceptable
tradeoff for this bug-fix PR. Can be revisited if large-output scenarios arise.
--
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]
