njnu-seafish commented on PR #18705:
URL:
https://github.com/apache/dolphinscheduler/pull/18705#issuecomment-6340008977
> The main purpose of the `limit` restriction here is to prevent a large
amount of data from being loaded into memory and causing the service to run out
of memory (OOM). Could we print the results directly to the log after the query
is executed, rather than caching all the data and outputting it in JSON format?
Hi @ruanwenjun, thanks for the comment!
Agreed — the row limit here is essentially an OOM guard rather than a
functional setting.
One thing to note before switching to log-only output: the returned JSON is
not used just for display, it has two existing consumers:
OUT parameters — dealOutParam(result) parses this JSON to fill the varPool,
so downstream tasks can reference the values via ${prop}; for LIST-typed OUT
parameters the whole column is passed down.
sendEmail — when enabled, the email content is built from the full result
(bounded by the sql.task.alert.max.* properties).
In those cases the full result has to stay in memory anyway, so printing
each row to the log and discarding it would break the downstream-parameter and
alert contracts.
That said, you're right that in the common case — no OUT parameters, no
sendEmail — today we are caching all the data and outputting it in JSON format
for nothing: nothing consumes it except a debug log. A middle ground could be
to buffer the result only when it actually has a consumer (OUT parameters or
sendEmail), and iterate the ResultSet forward without buffering otherwise,
printing at most displayRows rows to the log as now. That would remove the
memory spike the row limit is meant to guard, while the limit still protects
the cases that must buffer. Meanwhile the configurable default stays useful for
exactly those tasks.
If you think this is worth pursuing, I'd suggest doing it as a follow-up
issue/PR to keep this one focused on the configurability — but happy to go
either way, what do you prefer?
--
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]
