The remote site should see it connect, be open for 1 single stream and read the message before it's notified that the participant is closed actually.
This caused the faulure in one of our API tests (in lib, with client and server). Where it was possible that all messages were send and one side was dropped before the other side asked for the opened stream
Also introduce better error detection in participant(and scheduler) by removing the std_async::Result and intruduce `Result<(),ParticipantError>` instead
We MUST handle them and we are not allowed to act on a stream after it failed, as i am to lazy to change the structure to ensure the client to be imeadiatly dropped i added a AtomicBool to it.
- we had the problem that Participants couldn't shutdown them self, only by scheduler, which was controlled by api.
it's needed e.g. to handle the Schudown Frame
- my initial solution did a full shutdown, which was a problem if in parallel a 2nd shutdown was requested, no possibility of getting the error
- new solution will only deactivate Participant and Stream. and then still functions correctly, till the api closes the participant and calls the scheduler which then calls the bparticipant again
- i experimented with a Mutex<oneshot> or 2 and a `select` but it didn't prove that well
- also adjusted the Error messages to now either Disconnected when gracefully shutdown or ProtocolFailed when some msg couldn't be delivered
(note later might not be 100% returned correctly yet)
- make it harder for the server to crash and also kill invalid sessions properly (instead of waiting for them to close)
- introduce macros to reduce code duplication
- added tests to check for valid handshake as well as garbage tcp
- voxygen abort when the server has a invalid veloren_network handshake, e.g. by outdated version instead of try again
- rename Network `Address` to `ProtocolAddr` as sugested by zest as it's a combination of Protocol and std::io::Addr
- remove the manual byte arrays in `protocols.rs` with something more nice
- API behavior switched!
- the `Network` no longer holds a copy of participant, thus if the return of `connect` (before `Arc<Participant>, now `Participant`) got dropped, the `Participant::Drop` is triggered!
- you can close a Participant async via `Particiant::disconnect()`, no more need to know the network at this point
- the `Network::Drop` will check and drop not yet disconnected Participants.
- you can compare Participants via PartialEq, if they are true they point to the same endpoint (it checks remote_pid)
- Note: multiple Participants are only supported in theory, wont work yet
Additionally:
- fix some `debug!`
- veloren-client will now drop the participant gracefully on shutdown
- rename `error` to `debug` when 2 times Bparticipant shutdown is called, as it is to be expected in a async runtime
- swap out std::mpsc with crossbeam-channel in networking crate
- remove log spam by only logging when populating a new cache entry and not on every get
- switched the behavior of prio to keep the order of a stream, EVEN if messages of different length are used!
This needs to be addresses to fulfill PROMISES_ORDERED
I think we need to make thoughs if PRIO needs to handle this, or we add additional meta info and handle it on receiver site
- added PartialEq to StreamError for test purposes (only yet!)
- removed async_recv example as it's no longer for any use.
It was created before the COMPLETE REWRITE in order to verify that my own async interface on top of mio works.
However it's now guaranteed by async-std and futures. no need for a special test
- remove uvth from dependencies and replace it with a `FnOnce`
- fix ALL clippy (network) lints
- basic fix for a channel drop scenario:
TODO: this needs some further fixes
up to know only destruction of participant by api was covered correctly.
we had an issue when the underlying channels got dropped. So now we have a participant without channels.
We need to buffer the requests and try to reopen a channel ASAP!
If no channel could be reopened we need to close the Participant, while
a) leaving the BParticipant in takt, knowing that it only waits for a propper close by scheduler
b) close the BParticipant gracefully. Notifying the scheduler to remove its stuff (either scheduler schould detect a stopped BParticipant or BParticipant will send Scheduler it's own destruction, and then Scheduler just does the same like when API forces a close)
Keep the Participant alive and wait for the api to acces BParticipant to notice it's closed and then wait for a disconnect which isn't doing anything as it was already cleaned up in the background
- this bug was initially called imbris bug, as it happened on his runners and i couldn't reproduce it locally at fist :)
- When in a Handshake a seperate mpsc::Channel was created for (Cid, Frame) transport
however the protocol could already catch non handshake data any more and push in into this
mpsc::Channel.
Then this channel got dropped and a fresh one was created for the network::Channel.
These droped Frames are ofc a BUG!
I tried multiple things to solve this:
- dont create a new mpsc::Channel, but instead bind it to the Protocol itself and always use 1.
This would work theoretically, but in bParticipant side we are using 1 mpsc::Channel<(Cid, Frame)>
to handle ALL the network::channel.
If now ever Protocol would have it's own, and with that every network::Channel had it's own it would no longer work out
Bad Idea...
- using the first method but creating the mpsc::Channel inside the scheduler instead protocol neither works, as the
scheduler doesnt know the remote_pid yet
- i dont want a hack to say the protocol only listen to 2 messages and then stop no matter what
So i switched over to the simply method now:
- Do everything like before with 2 mpsc::Channels
- after the handshake. close the receiver and listen for all remaining (cid, frame) combinations
- when starting the channel, reapply them to the new sender/listener combination
- added tracing
- switched Protocol RwLock to Mutex, as it's only ever 1
- Additionally changed the layout and introduces the c2w_frame_s and w2s_cid_frame_s name schema
- Fixed a bug in scheduler which WOULD cause a DEADLOCK if handshake would fail
- fixd a but in api_send_send_main, i need to store the stream_p otherwise it's immeadiatly closed and a stream_a.send() isn't guaranteed
- add extra test to verify that a send message is received even if the Stream is already closed
- changed OutGoing to Outgoing
- fixed a bug that `metrics.tick()` was never called
- removed 2 unused nightly features and added `deny_code`
fix async_recv and double block_on panic on Network::drop and participant::drop
include Cargo.lock from all examples
Found a bug on imbris runners with doc tests of `stream::send` and `stream::recv`
As neither a backtrace, nor tracing on runners in the doc tests seems to help, i disable them and add them as unit tests
however i need to coordinate the prio adjustments in scheduler from now on, so that ParticipantA doesn't get all the network bandwith and ParticipantB nothing
- https://github.com/tikv/rust-prometheus/issues/321
- split up channel into a hanshake part and channel part.
The handshake part is non endless and ends when its either done or aborted.
If its okay i will send a request to the BParticipant which then opens a channel on the existing TCP or UDP connection.
this streamlines the command chain alot. also the channel is almost empty now, thinking about removing it completly.
isnt perfect, as shutdown and udp doesnt work yet
- make PID to print as Base64
- replace rouille with tiny_http
- removing async_serde as it seems to be not usefull
the idea was because deserialising is slow parallising it could speed up.
Whoever we need to keep the order of frames, (at least for controlframes) so serialising in threads would be quite complicated.
Also serialisation is quite fast, about 1 Gbit/s such speed is enough for messaging, it's more important to serve parallel streams better.
Thats why i am removing async serde coding for now
- frames are no longer serialized by serde, by byte by byte manually, increadible speed upgrade
- more metrics
- switch channel_creator into for_each_concurrent
- removing some pool.spwan_ok() as they dont allow me to use self
- reduce features needed
- switch `listen` to async in oder to verify if the bind was successful
- Introduce the following examples
- network speed
- chat
- fileshare
- add additional tests
- fix dropping stream before last messages can be handled bug, when dropping a stream, BParticipant will wait for prio to be empty before dropping the stream and sending the signal
- correct closing of stream and participant
- move tcp to protocols and create udp front and backend
- tracing and fixing a bug that is caused by not waiting for configuration after receiving a frame
- fix a bug in network-speed, but there is still a bug if trace=warn after 2.000.000 messages the server doesnt get that client has shut down and seems to lock somewhere. hard to reproduce
open tasks
[ ] verify UDP works correctly, especcially the connect!
[ ] implements UDP shutdown correctly, the one created in connect!
[ ] unify logging
[ ] fill metrics
[ ] fix dropping stream before last messages can be handled bug
[ ] add documentation
[ ] add benchmarks
[ ] remove async_serde???
[ ] add mpsc
- We can now get rid of most sleep and get true remote part and stream working, however there seems to be a deadlock after registered new handle trace with 10% spawn chance
- removal of the events trait, as we use channels
- streams now directly communicate with each other for performance reasons, somewhere are still deadlocks, oonce directly at listening somehow and after the first message has read, but i also got it to run perfectly through at this state without code change, maybe a sleep or more detailed rust-dgb session would help here!
