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

fix(hdb): Allow streaming blob in and out over the same connection #561

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

BobdenOs
Copy link
Contributor

@BobdenOs BobdenOs commented Apr 2, 2024

Currently it is possible to have draft operations hang when using the hdb driver. As the draft logic will stream blob values in/out of the database over the same connection. When using the hdb driver the insert statement is claiming the connection and all readLob requests are queued until the insert statement finishes. It will never finish as the contents of the blob have to be send as part of the insert statement.

This PR avoids this issue by opening the connection queue when the connection is actively reading a blob stream. This works as the insert statement claims the connection before it starts using the connection. It will first load as much data of the parameter stream into memory before sending it as a packet over the connection.

This is a workaround and for it to work all the time has to be addressed inside the node-hdb repository.

@soccermax
Copy link
Contributor

@BobdenOs can we also try to fix this in HDB? Because with this solution we would have for lobs always two database connections. What do you think?

@BobdenOs
Copy link
Contributor Author

BobdenOs commented Apr 2, 2024

@soccermax the way hdb has setup its connection queue makes it a fundamental part of the library. It might be possible to have the insert return early just like hana-client, but I was not able to pinpoint where to do this. So rather then putting to much time into investigating that option. I wanted to have a possible solution first.

Copy link
Member

@patricebender patricebender left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, is this still relevant?

do we need this first?:

This is a workaround and for it to work all the time has to be addressed inside the node-hdb repository.

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

Successfully merging this pull request may close these issues.

3 participants