Benph/bs estab de async heavy draft #3633

Ben-PH · 2023-03-02T15:31:46Z

Is here to use github as a tool to setup a checklist of things to address...

In the future, it might just be better to use a personal fork :/

Ben-PH

Self-review/checklist of things that need attention.

Ben-PH · 2023-03-02T15:36:26Z

massa-bootstrap/src/client.rs

-async fn stream_final_state_and_consensus(
+fn stream_final_state_and_consensus(


simple call

Ben-PH · 2023-03-02T15:37:19Z

massa-bootstrap/src/client.rs

-        match tokio::time::timeout(
-            cfg.write_timeout.into(),
-            client.send(next_bootstrap_message),
-        )
-        .await
-        {
-            Err(_) => Err(std::io::Error::new(
-                std::io::ErrorKind::TimedOut,
-                "bootstrap ask ledger part send timed out",
-            )
-            .into()),
-            Ok(Err(e)) => Err(e),
-            Ok(Ok(_)) => Ok(()),
-        }?;
+        let _ask_ledger_elapsed = client
+            .blocking_send(next_bootstrap_message, Some(cfg.write_timeout.into()))
+            .map_err(|e| e.update_io_msg_payload("bootstrap ask ledger part send timed out"))?;


Ben-PH · 2023-03-02T15:38:00Z

massa-bootstrap/src/client.rs

-            let msg = match tokio::time::timeout(cfg.read_timeout.into(), client.next()).await {
-                Err(_) => {
-                    return Err(std::io::Error::new(
-                        std::io::ErrorKind::TimedOut,
-                        "final state bootstrap read timed out",
-                    )
-                    .into());
-                }
-                Ok(Err(e)) => return Err(e),
-                Ok(Ok(msg)) => msg,
-            };
+            let (msg, _next_elapsed) =
+                client
+                    .blocking_next(Some(cfg.read_timeout.to_duration()))
+                    .map_err(|e| e.update_io_msg_payload("final state bootstrap read timed out"))?;


Ben-PH · 2023-03-02T15:38:42Z

massa-bootstrap/src/client.rs

-/// needs to be CANCELLABLE
-async fn bootstrap_from_server(
+/// TODO: setup cancellations
+fn bootstrap_from_server(


This needs attention

Ben-PH · 2023-03-02T15:39:41Z

massa-bootstrap/src/client.rs

-    match tokio::time::timeout(cfg.read_error_timeout.into(), client.next()).await {
-        Err(_) => {
-            massa_trace!(
-                "bootstrap.lib.bootstrap_from_server: No error sent at connection",
-                {}
-            );
+    let next_tollerance = cfg.read_error_timeout.to_duration();
+    let res = client.blocking_next(Some(next_tollerance));
+    match res {
+        Err(BootstrapError::IoError(ref e)) => {
+            if e.kind() == ErrorKind::TimedOut {
+                massa_trace!(
+                    "bootstrap.lib.bootstrap_from_server: No error sent at connection",
+                    {}
+                );
+            } else {
+                return Err(res.unwrap_err());
+            }
        }
-        Ok(Err(e)) => return Err(e),
-        Ok(Ok(BootstrapServerMessage::BootstrapError { error: err })) => {
-            return Err(BootstrapError::ReceivedError(err))
+        Err(e) => {
+            return Err(e);
        }
-        Ok(Ok(msg)) => return Err(BootstrapError::UnexpectedServerMessage(msg)),
-    };
+        Ok((BootstrapServerMessage::BootstrapError { error }, _)) => {
+            return Err(BootstrapError::ReceivedError(error.to_string()));
+        }
+        Ok((msg, _)) => return Err(BootstrapError::UnexpectedServerMessage(msg)),
+    }


look at this side-by-side

Ben-PH · 2023-03-02T16:00:11Z

massa-bootstrap/src/server.rs

-        )
-        .await
-    {
-        Err(_) => Err(std::io::Error::new(
-            std::io::ErrorKind::TimedOut,
-            "bootstrap clock send timed out",
-        )
-        .into()),
-        Ok(Err(e)) => Err(e),
-        Ok(Ok(_)) => Ok(()),
-    }?;
+    let fetch_time = server.send_msg(
+        BootstrapServerMessage::BootstrapTime {
+            server_time,
+            version,
+        },
+        Some(write_timeout),
+    )?;
+    // {
+    //     Err(_) => Err(std::io::Error::new(
+    //         std::io::ErrorKind::TimedOut,
+    //         "bootstrap clock send timed out",
+    //     )
+    //     .into()),
+    //     Ok(Err(e)) => Err(e),
+    //     Ok(Ok(_)) => Ok(()),
+    // }?;


is this right?

Ben-PH · 2023-03-02T16:00:24Z

massa-bootstrap/src/server.rs

-                            },
-                        )
-                        .await
-                    {
-                        Err(_) => Err(std::io::Error::new(
-                            std::io::ErrorKind::TimedOut,
-                            "bootstrap peers send timed out",
-                        )
-                        .into()),
-                        Ok(Err(e)) => Err(e),
-                        Ok(Ok(_)) => Ok(()),
-                    }?;
-                }
-                BootstrapClientMessage::AskBootstrapPart {
-                    last_slot,
-                    last_ledger_step,
-                    last_pool_step,
-                    last_cycle_step,
-                    last_credits_step,
-                    last_ops_step,
-                    last_consensus_step,
-                } => {
-                    stream_bootstrap_information(
-                        server,
-                        final_state.clone(),
-                        consensus_controller.clone(),
-                        last_slot,
-                        last_ledger_step,
-                        last_pool_step,
-                        last_cycle_step,
-                        last_credits_step,
-                        last_ops_step,
-                        last_consensus_step,
-                        write_timeout,
-                    )
-                    .await?;
-                }
-                BootstrapClientMessage::BootstrapSuccess => break Ok(()),
-                BootstrapClientMessage::BootstrapError { error } => {
-                    break Err(BootstrapError::ReceivedError(error));
-                }
-            },
-        };
+        let next = server.blocking_next(Some(bootstrap_config.read_timeout.into()));
+        todo!("handle next");
+        // {
+        //     Err(_) => break Ok(()),
+        //     Ok(Err(e)) => break Err(e),
+        //     Ok(Ok(msg)) => match msg {
+        //         BootstrapClientMessage::AskBootstrapPeers => {
+        //             match server
+        //                 .send_msg(
+        //                     write_timeout,
+        //                     BootstrapServerMessage::BootstrapPeers {
+        //                         peers: network_command_sender.get_bootstrap_peers().await?,
+        //                     },
+        //                 )
+        //                 .await
+        //             {
+        //                 Err(_) => Err(std::io::Error::new(
+        //                     std::io::ErrorKind::TimedOut,
+        //                     "bootstrap peers send timed out",
+        //                 )
+        //                 .into()),
+        //                 Ok(Err(e)) => Err(e),
+        //                 Ok(Ok(_)) => Ok(()),
+        //             }?;
+        //         }
+        //         BootstrapClientMessage::AskBootstrapPart {
+        //             last_slot,
+        //             last_ledger_step,
+        //             last_pool_step,
+        //             last_cycle_step,
+        //             last_credits_step,
+        //             last_ops_step,
+        //             last_consensus_step,
+        //         } => {
+        //             stream_bootstrap_information(
+        //                 server,
+        //                 final_state.clone(),
+        //                 consensus_controller.clone(),
+        //                 last_slot,
+        //                 last_ledger_step,
+        //                 last_pool_step,
+        //                 last_cycle_step,
+        //                 last_credits_step,
+        //                 last_ops_step,
+        //                 last_consensus_step,
+        //                 write_timeout,
+        //             )
+        //             .await?;
+        //         }
+        //         BootstrapClientMessage::BootstrapSuccess => break Ok(()),
+        //         BootstrapClientMessage::BootstrapError { error } => {
+        //             break Err(BootstrapError::ReceivedError(error));
+        //         }
+        //     },
+        // };
    }
+    todo!()


!!!!!!!

Ben-PH · 2023-03-02T16:00:36Z

massa-bootstrap/src/server_binder.rs

-            duplex: <Limiter>::new(limit).limit(duplex),
+            duplex,


Ben-PH · 2023-03-02T16:01:03Z

massa-bootstrap/src/server_binder.rs

-    pub(crate) fn close_and_send_error<F>(
-        mut self,
-        server_outer_rt_hnd: Handle,
-        msg: String,
-        addr: SocketAddr,
-        close_fn: F,
-    ) where
+    pub(crate) fn close_and_send_error<F>(mut self, msg: String, addr: SocketAddr, close_fn: F)
+    where


probably deserves to live in a poll?

Ben-PH · 2023-03-02T16:01:53Z

massa-node/src/main.rs

-        ) => match res {
+        )
+    });
+    // bootstrap
+    let bootstrap_state = crossbeam_channel::select! {
+        //TODO: intercept a ctrl-c
+        // _ = &mut stop_signal => {
+        //     info!("interrupt signal received in bootstrap loop");
+        //     process::exit(0);
+        // },
+        recv(state_rx) -> res => match res {


surely mio can manage this?

modship · 2023-03-03T09:03:02Z

massa-bootstrap/Cargo.toml

@@ -40,6 +36,7 @@ massa_signature = { path = "../massa-signature" }
 massa_pos_exports = { path = "../massa-pos-exports" }
 massa_time = { path = "../massa-time" }
 crossbeam = "0.8.2"
+mio = {version = "0.8.6", features = ["os-poll", "net"]}


mio && crossbeam should be in dependencies not "custom modules"

Thanks for the catch!

modship · 2023-03-03T09:29:51Z

massa-bootstrap/src/server_binder.rs

+        let mut acc = 0;
+        let mut remain = time_limit;
+        let mut clock = Duration::from_secs(0);
+        while acc < buf.len() {


we can improve a bit that with :

let mut acc = 0; while acc < buf.len() { if Instant::now().duration_since(start) > time_limit { return Err((acc, std::io::ErrorKind::TimedOut.into())); } // self.duplex // .set_read_timeout(Some(time_limit - clock)) // .expect("internal error"); self.duplex.read(&mut buf[acc..]).map_err(|er| (acc, er))?; }

that remove remain, clock and his alloc

modship · 2023-03-03T10:23:56Z

massa-bootstrap/src/server_binder.rs

-    ) -> Result<Result<(), BootstrapError>, Elapsed> {
-        tokio::time::timeout(timeout, self.send(msg)).await
+        timeout: Option<Duration>,
+    ) -> Result<Duration, BootstrapError> {


i don't know if it's useful to return the Duration

modship · 2023-03-03T10:40:58Z

massa-bootstrap/src/tests/tools.rs

@@ -301,7 +301,10 @@ pub fn get_bootstrap_config(bootstrap_public_key: NodeId) -> BootstrapConfig {
        write_timeout: 1000.into(),
        read_error_timeout: 200.into(),
        write_error_timeout: 200.into(),
-        bootstrap_list: vec![(SocketAddr::new(BASE_BOOTSTRAP_IP, 16), bootstrap_public_key)],
+        bootstrap_list: vec![(
+            SocketAddr::new(BASE_BOOTSTRAP_IP, 8069),


why port 8069

16 was broken before. this change fixed it. had to pick one at random.

Ben-PH · 2023-03-30T14:36:49Z

closed in favor of #3685 and assonciated PRs

Ben-PH and others added 16 commits February 28, 2023 15:28

Make establisher get_listener non-async

e96b831

Make establisher fn get_connector non-async

5d8722d

Set the mockers to use a TcpStream as a duplex

8b91f0a

fix(bootstrap) : fix base_bootstrap_ip

a636afd

Make duplex std::net::TcpStream

c025451

DefaultListener::accept() -> blocking_accept()

eac9eba

Make connect method non-async with built-in timeout management

f9b31fd

Continue work on having a timeout story for our stream reading/writing

5650ec1

cargo check passes

9cf4499

Add ergonomic payload setting for bootstrap io errors

bddd510

Pass up io message payload for stream_final_state_and_consensus

7d15218

Refine client.rs code when calling blocking_next/send

6db5f46

Adapt crossbeam::select! to work with WouldBlock OS calls

f561249

Bring client.rs closer to being a 1-1 refactor

6c0154e

Clear redundant async stuff, clear some warnings

ee8d892

Use mio to poll for an incomming connection.

56944cd

Ben-PH added bootstrap Issues related to the bootstrap refactoring labels Mar 2, 2023

Ben-PH self-assigned this Mar 2, 2023

Ben-PH commented Mar 2, 2023

View reviewed changes

modship reviewed Mar 3, 2023

View reviewed changes

Ben-PH mentioned this pull request Mar 3, 2023

Refactor mock-systems to not need #[cfg(test)], new-types, or diverging types #3639

Closed

Ben-PH closed this Mar 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Benph/bs estab de async heavy draft #3633

Benph/bs estab de async heavy draft #3633

Ben-PH commented Mar 2, 2023 •

edited

Loading

Ben-PH left a comment

Ben-PH Mar 2, 2023

Ben-PH Mar 2, 2023

Ben-PH Mar 2, 2023

Ben-PH Mar 2, 2023

Ben-PH Mar 2, 2023

Ben-PH Mar 2, 2023

Ben-PH Mar 2, 2023

Ben-PH Mar 2, 2023

Ben-PH Mar 2, 2023

Ben-PH Mar 2, 2023

modship Mar 3, 2023

Ben-PH Mar 3, 2023

modship Mar 3, 2023

modship Mar 3, 2023

modship Mar 3, 2023

Ben-PH Mar 3, 2023

Ben-PH commented Mar 30, 2023

		async fn stream_final_state_and_consensus(
		fn stream_final_state_and_consensus(

Benph/bs estab de async heavy draft #3633

Benph/bs estab de async heavy draft #3633

Conversation

Ben-PH commented Mar 2, 2023 • edited Loading

Ben-PH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ben-PH commented Mar 30, 2023

Ben-PH commented Mar 2, 2023 •

edited

Loading