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

⚠️ This does not handle unbuffered cases. #3

Open
jkcs opened this issue Dec 28, 2023 · 9 comments
Open

⚠️ This does not handle unbuffered cases. #3

jkcs opened this issue Dec 28, 2023 · 9 comments

Comments

@jkcs
Copy link
Contributor

jkcs commented Dec 28, 2023

⚠️ This does not handle unbuffered cases.

Originally posted by @ohsayan in #2 (comment)

@jkcs
Copy link
Contributor Author

jkcs commented Dec 28, 2023

@ohsayan Do you have a reproducible statement? I don't understand the specific problem situation.

@ohsayan
Copy link
Member

ohsayan commented Dec 29, 2023

@jkcs the current connectionWrite function does write data to the socket correctly but has the primary problem of assuming that enough data for the response has already been buffered in the socket's read buffer.

For slightly larger responses, this will not be the case and the function will return an error. If you see the Rust driver for example, it checks if sufficient data is buffered by the socket in a loop and if not, it continues to wait for more data.

@jkcs
Copy link
Contributor Author

jkcs commented Dec 29, 2023

@ohsayan In other words, when I send a query, is it possible for the Skytable server to respond multiple times?

@ohsayan
Copy link
Member

ohsayan commented Dec 29, 2023

@ohsayan In other words, when I send a query, is it possible for the Skytable server to respond multiple times?

The server doesn't "respond multiple times" -- it only responds once! But the response when large enough will take longer to be transported over the network.

So essentially when you do a read syscall all the data might not have been sent and you'll have to wait for all of it to reach you. That's how it works for all servers -- even your HTTP responses are read in a loop under the hood!

@ohsayan
Copy link
Member

ohsayan commented Dec 29, 2023

The easiest way to understand it is like this — imagine you’re sending over a 5GB file; you will never receive all of it at once and hence you’ll keep polling to see if you received any new data and then update your local file accordingly.

Also, there is no end of transmission signal because of binary data (since a last byte can easily coincide with a signal — and unnecessarily complex logic would have to be added for a end of transmission block).

In a way, Skyhash is kind of like a streaming protocol.

@jkcs
Copy link
Contributor Author

jkcs commented Dec 29, 2023

I see where the problem lies, and I'm envisioning a solution.

@ohsayan
Copy link
Member

ohsayan commented Dec 29, 2023

You can take a look at what the Rust driver does. I already have a somewhat draft implementation for the NodeJS client.

@jkcs
Copy link
Contributor Author

jkcs commented Dec 29, 2023

@ohsayan
Copy link
Member

ohsayan commented Dec 29, 2023

Yeah, that's what I did with the Rust implementation. A similar approach would be plausible

This was referenced Dec 29, 2023
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 a pull request may close this issue.

2 participants