Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

createRowStream is suboptimal #12

Open
vweevers opened this issue Jun 6, 2018 · 0 comments
Open

createRowStream is suboptimal #12

vweevers opened this issue Jun 6, 2018 · 0 comments

Comments

@vweevers
Copy link
Owner

vweevers commented Jun 6, 2018

Because it sets highWaterMark to 0 (though documented as 16):

lento/lib/client.js

Lines 176 to 180 in 8670652

createRowStream (sql, opts) {
const pageOpts = Object.assign({}, opts, {
paging: false,
highWaterMark: 0,
})

This makes sense for a page stream, because it means it won't make preemptive HTTP requests. But for
a row stream it means it'll push only one row (or "chunk") at a time, possibly hurting throughput:

// Push single rows
while (this._buffer !== undefined && this._bufferIndex < this._buffer.length) {
const chunk = this._buffer[this._bufferIndex++]
if (!this.push(chunk)) {
this._reading = false
return
}
}

I.e. the while loop above never loops. It might be easier to push everything at once (ignoring the return value of push). It's already occupying memory anyway.

Also, when the next readable-stream is out (with nodejs/node@1e0f331), I can maybe get rid of:

// See https://github.com/nodejs/node/issues/3203
if (this._reading) return
else this._reading = true

For now, I recommend to use createPageStream instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant