Add Rust server and client for `get_simple` example #8

mbrobbel · 2024-03-07T15:55:16Z

A simple synchronous Rust (std-based) server and client for the get_simple example.

This skips over all http request/response parsing and error handling, but the examples can be updated later to use hyper (when there is async Arrow IPC support in arrow-rs).

❯ cargo r -p server --release
    Finished release [optimized] target(s) in 0.04s
     Running `target/release/server`
2024-03-07T20:14:47.266470Z  INFO data{TOTAL_RECORDS=100000000 RECORDS_PER_BATCH=4096}: server: Generating random data
2024-03-07T20:14:48.109722Z  INFO data{TOTAL_RECORDS=100000000 RECORDS_PER_BATCH=4096}: server: close time.busy=843ms time.idle=1.12µs
2024-03-07T20:14:48.109820Z  INFO server: Listening bind_addr=127.0.0.1:8000
2024-03-07T20:15:17.403848Z  INFO Writing Arrow IPC stream{remote_peer=127.0.0.1:50291}: server: Incoming connection
2024-03-07T20:15:18.006722Z  INFO Writing Arrow IPC stream{remote_peer=127.0.0.1:50291}: server: close time.busy=603ms time.idle=2.08µs

❯ cargo r -p client --release
    Finished release [optimized] target(s) in 0.03s
     Running `target/release/client`
2024-03-07T20:15:17.403862Z  INFO Reading Arrow IPC stream{addr=127.0.0.1:8000}: client: Connected
2024-03-07T20:15:18.007890Z  INFO Reading Arrow IPC stream{addr=127.0.0.1:8000}: client: batches=24415 rows=100000000
2024-03-07T20:15:18.147404Z  INFO Reading Arrow IPC stream{addr=127.0.0.1:8000}: client: close time.busy=744ms time.idle=1.21µs

ianmcook · 2024-03-07T19:42:27Z

Thanks @mbrobbel !

Does the Cargo.lock file need to be committed here?

mbrobbel · 2024-03-07T19:57:31Z

Does the Cargo.lock file need to be committed here?

It's the recommended approach:

ianmcook · 2024-03-07T21:48:00Z

It's the recommended approach

After reading the guidance on this from the Cargo team, I think I'm -1 on including it in this case. The project here is intended to serve as an example for other projects to build on, not as a standalone project where the top priority is for the binary to build without errors. It seems undesirable in this case to have 900+ extra lines just to ensure the latter. The spirit of this area of the repo is to keep everything very minimal and not have any unnecessary assets even if they are black boxes.

ianmcook · 2024-03-08T14:42:32Z

I tested this Rust server with the clients implemented in other languages. That worked fine.

But I ran into problems when I tested this Rust client with the servers implemented in other languages.

http/get_simple/rs/client/src/main.rs

ianmcook · 2024-03-08T17:49:17Z

The client works with the Python server example now, but I see errors when I try to use it with the Java and Go server examples.

With the Java server, I see this error:

thread 'main' panicked at client/src/main.rs:52:77:
called `Result::unwrap()` on an `Err` value: ParseError("Unable to get root as message: Unaligned { position: 3795177799, unaligned_type: \"i32\", error_trace: ErrorTrace([]) }")

With the Go server, I see this error:

thread 'main' panicked at library/alloc/src/raw_vec.rs:571:5:
capacity overflow

cc @zeroshade

mbrobbel · 2024-03-08T18:41:53Z

The client works with the Python server example now, but I see errors when I try to use it with the Java and Go server examples.

With the Java server, I see this error:
thread 'main' panicked at client/src/main.rs:52:77:
called `Result::unwrap()` on an `Err` value: ParseError("Unable to get root as message: Unaligned { position: 3795177799, unaligned_type: \"i32\", error_trace: ErrorTrace([]) }")
With the Go server, I see this error:
thread 'main' panicked at library/alloc/src/raw_vec.rs:571:5:
capacity overflow
cc @zeroshade

The problem is caused by the BufReader in the client. In the case of the errors it buffered some bytes from the beginning of the body. I'll fix it.

mbrobbel · 2024-03-08T18:55:02Z

Do we want CI?

ianmcook · 2024-03-08T19:56:37Z

Do we want CI?

Not yet. We will want to add CI later, once the examples like these become part of an official conventions document on the Arrow docs site. I think in general we want to keep this repo free of CI.

ianmcook

I tested these with all the other client and server examples and they work as expected. Thanks very much @mbrobbel!

@wjones127 or anyone else watching here: is there anything else you'd like to see here before we merge this?

ianmcook · 2024-03-10T05:54:04Z

@mbrobbel I'm curious how feasible it would be to make the client example handle HTTP/1.1 responses with chunked transfer encoding? I think this would just involve handling lines with \r\n a bit differently.

ianmcook

Thanks @mbrobbel! I re-tested this new version that includes chunked encoding support against all the other server examples, toggling chunked encoding on and off on the servers, and it works as expected.

ianmcook · 2024-03-11T14:07:47Z

I will merge later today if there are no other comments.

wjones127

Seems okay, though I would definitely prefer it written in something higher-level like hyper. As-is, I'm not sure how many developers would use this logic.

wjones127 · 2024-03-11T17:13:58Z

http/get_simple/rs/client/src/main.rs

+                    let mut chunked = false;
+                    loop {
+                        let mut line = String::default();
+                        reader.read_line(&mut line).unwrap();
+                        if let Some(("transfer-encoding", "chunked")) = line
+                            .to_lowercase()
+                            .split_once(':')
+                            .map(|(key, value)| (key.trim(), value.trim()))
+                        {
+                            chunked = true;
+                        }
+                        if line == "\r\n" {
+                            break;
+                        }
+                    }


It feels a bit sad we are manually parsing headers and such. Makes the example a bit harder to follow.

100% agreed. I started with hyper but moved to std because of apache/arrow-rs#1207.

Agreed. Of all the examples here, this is the only one that directly implements low-level HTTP details instead of depending on a library to do that.

But the main objective of these get_simple examples is only to ensure that the IPC implementations in the mainstream Arrow libraries can function and interoperate over HTTP. This achieves that objective.

http/get_simple/rs/client/README.md

http/get_simple/rs/server/README.md

Co-authored-by: Ian Cook <[email protected]>

tustvold · 2024-03-19T06:24:16Z

FYI with apache/arrow-rs#5531 it should be possible to switch this to use hyper

Add Rust server and client for get_simple example

b95a780

Remove and ignore Cargo.lock

c35d903

mbrobbel added 2 commits March 8, 2024 17:08

Fix get request in client

94241f5

Merge remote-tracking branch 'upstream/main' into rust

8ab740c

ianmcook reviewed Mar 8, 2024

View reviewed changes

http/get_simple/rs/client/src/main.rs Show resolved Hide resolved

Wrap span around the full example

76d8c24

Re-use the BufReader

292b304

mbrobbel marked this pull request as ready for review March 8, 2024 18:51

ianmcook approved these changes Mar 8, 2024

View reviewed changes

ianmcook mentioned this pull request Mar 9, 2024

Show how to disable chunked transfer encoding in Go and Java server examples #13

Merged

Handle chunked transfer encoding in client

b729bf6

ianmcook approved these changes Mar 11, 2024

View reviewed changes

wjones127 approved these changes Mar 11, 2024

View reviewed changes

ianmcook reviewed Mar 11, 2024

View reviewed changes

http/get_simple/rs/client/README.md Show resolved Hide resolved

ianmcook reviewed Mar 11, 2024

View reviewed changes

http/get_simple/rs/server/README.md Show resolved Hide resolved

Apply suggestions from code review

fe6ebcf

Co-authored-by: Ian Cook <[email protected]>

mbrobbel mentioned this pull request Mar 11, 2024

Prototype Arrow over HTTP in Rust apache/arrow-rs#5496

Closed

ianmcook merged commit bbbfaf6 into apache:main Mar 11, 2024

mbrobbel deleted the rust branch March 11, 2024 19:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Rust server and client for `get_simple` example #8

Add Rust server and client for `get_simple` example #8

mbrobbel commented Mar 7, 2024 •

edited

Loading

ianmcook commented Mar 7, 2024

mbrobbel commented Mar 7, 2024 •

edited

Loading

ianmcook commented Mar 7, 2024

ianmcook commented Mar 8, 2024

ianmcook commented Mar 8, 2024 •

edited

Loading

mbrobbel commented Mar 8, 2024 •

edited

Loading

mbrobbel commented Mar 8, 2024

ianmcook commented Mar 8, 2024

ianmcook left a comment

ianmcook commented Mar 10, 2024

ianmcook left a comment

ianmcook commented Mar 11, 2024

wjones127 left a comment

wjones127 Mar 11, 2024

mbrobbel Mar 11, 2024

ianmcook Mar 11, 2024

tustvold commented Mar 19, 2024

Add Rust server and client for get_simple example #8

Add Rust server and client for get_simple example #8

Conversation

mbrobbel commented Mar 7, 2024 • edited Loading

ianmcook commented Mar 7, 2024

mbrobbel commented Mar 7, 2024 • edited Loading

ianmcook commented Mar 7, 2024

ianmcook commented Mar 8, 2024

ianmcook commented Mar 8, 2024 • edited Loading

mbrobbel commented Mar 8, 2024 • edited Loading

mbrobbel commented Mar 8, 2024

ianmcook commented Mar 8, 2024

ianmcook left a comment

Choose a reason for hiding this comment

ianmcook commented Mar 10, 2024

ianmcook left a comment

Choose a reason for hiding this comment

ianmcook commented Mar 11, 2024

wjones127 left a comment

Choose a reason for hiding this comment

wjones127 Mar 11, 2024

Choose a reason for hiding this comment

mbrobbel Mar 11, 2024

Choose a reason for hiding this comment

ianmcook Mar 11, 2024

Choose a reason for hiding this comment

tustvold commented Mar 19, 2024

Add Rust server and client for `get_simple` example #8

Add Rust server and client for `get_simple` example #8

mbrobbel commented Mar 7, 2024 •

edited

Loading

mbrobbel commented Mar 7, 2024 •

edited

Loading

ianmcook commented Mar 8, 2024 •

edited

Loading

mbrobbel commented Mar 8, 2024 •

edited

Loading