Implement re_grpc_server
#8622
Implement re_grpc_server
#8622
Conversation
|
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
jprochazk
added
the
🪵 Log & send APIs
jprochazk
force-pushed
the
jan/grpc-server
branch
from
329eef2 to
f59825a
Compare
jprochazk
force-pushed
the
jan/grpc-server
branch
from
c4a3008 to
2fb22b9
Compare
| @@ -0,0 +1,370 @@ | |||
| // This file is @generated by prost-build. | |||
There was a problem hiding this comment.
Then it should be marked as linguist-generated=true in a .gitattributes file!
There was a problem hiding this comment.
It seems like it is? All files in src/v0 should have that attribute already. I think GitHub is just confused, and is showing the "large diff" warning instead of "generated file" warning.
$ git check-attr -a -- crates/store/re_protos/src/v0/rerun.common.v0.rs
crates/store/re_protos/src/v0/rerun.common.v0.rs: text: auto
crates/store/re_protos/src/v0/rerun.common.v0.rs: eol: lf
crates/store/re_protos/src/v0/rerun.common.v0.rs: linguist-generated: true
$ git check-attr -a -- crates/store/re_protos/src/v0/rerun.sdk_comms.v0.rs
crates/store/re_protos/src/v0/rerun.sdk_comms.v0.rs: text: auto
crates/store/re_protos/src/v0/rerun.sdk_comms.v0.rs: eol: lf
crates/store/re_protos/src/v0/rerun.sdk_comms.v0.rs: linguist-generated: true
| let store_id = StoreId::random(StoreKind::Blueprint); | ||
|
|
||
| let mut messages = Vec::new(); | ||
| messages.push(LogMsg::SetStoreInfo(SetStoreInfo { |
There was a problem hiding this comment.
Wouldn't it make sense to have this after the ArrowMsg:s to test that it still arrives first for late-connecting client? 🤔 (saying this without having read any further than this)
There was a problem hiding this comment.
I think this is already being tested, but implicitly. SetStoreInfo goes into persistent_message_queue, which is then merged back into ordered_message_queue by prepending it in the resulting stream. So if it wasn't always being sent first, it wouldn't have the same order as it does in fake_log_stream. I'll add a comment about that.
| Message(LogMsgProto), | ||
| } | ||
|
|
||
| struct QueueState { |
There was a problem hiding this comment.
I wonder about the naming of Queue and QueueState below. We have spawn on one and run_in_place on the other, so I wonder if there's some better names for them as they are quite generic.
Queue seems to be responsible for 1/ passing messages to the underlying queue state and 2/ creating client streams.
This all is not terribly important as the code and especially logic in QueueState is quite clear, but I just wonder if there's actually value in having Queue at all and if that logic should be on the MessageProxy. And then perhaps renaming QueueState to MemoryBoundEventLoop (just the first thing that came to mind) would also help.
(just a thought, feel free to ignore if you don't think it makes sense)
There was a problem hiding this comment.
I think you're right, I inlined Queue into MessageProxy and renamed QueueState.
Related
re_grpc_server#8451What
This PR introduces a new gRPC service which behaves similarly to our existing TCP and WS comms.
WriteMessagesrpc will be stored in a buffer, and broadcast to all callers ofReadMessages.ReadMessageswill also yield the full history.server_memory_limit, then messages are dropped from history in oldest first order (FIFO).