wangrunji0408 commented on code in PR #2006:
URL: 
https://github.com/apache/incubator-opendal/pull/2006#discussion_r1169443844


##########
core/Cargo.toml:
##########
@@ -163,6 +166,7 @@ http = "0.2.5"
 hyper = "0.14"
 lazy-regex = { version = "2.5.0", optional = true }
 log = "0.4"
+madsim = {version = "0.2.19", optional = true }

Review Comment:
   use the latest version as it contains various fixes.
   ```suggestion
   madsim = { version = "0.2.21", optional = true }
   ```



##########
core/src/layers/madsim.rs:
##########
@@ -0,0 +1,316 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ops::{OpList, OpRead, OpScan, OpWrite};
+use crate::raw::oio::Entry;
+use crate::raw::{oio, Accessor, Layer, LayeredAccessor, RpList, RpRead, 
RpScan, RpWrite};
+use crate::{EntryMode, Metadata};
+use async_trait::async_trait;
+use bytes::Bytes;
+use madsim::plugin::simulator;
+use madsim::plugin::Simulator;
+use madsim::rand::GlobalRng;
+use madsim::runtime::Handle;
+use madsim::task::NodeId;
+use madsim::time::TimeHandle;
+use madsim::Config;
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::io::SeekFrom;
+use std::sync::{Arc, Mutex};
+use std::task::{Context, Poll};
+
+#[derive(Debug, Copy, Clone)]
+pub struct MadsimLayer {
+    service_sim_node: NodeId,
+}
+
+impl MadsimLayer {
+    pub fn new() -> Self {
+        let service_sim_node = Handle::current().create_node().build().id();
+        Self { service_sim_node }
+    }
+}
+
+impl<A: Accessor> Layer<A> for MadsimLayer {
+    type LayeredAccessor = MadsimAccessor;
+
+    fn layer(&self, _inner: A) -> Self::LayeredAccessor {
+        let handle = simulator::<ServiceSim>().get_node(self.service_sim_node);
+        MadsimAccessor { handle }
+    }
+}
+
+#[derive(Debug)]
+pub struct MadsimAccessor {
+    handle: ServiceNodeHandle,
+}
+
+#[async_trait]
+impl LayeredAccessor for MadsimAccessor {
+    type Inner = ();
+    type Reader = MadsimReader;
+    type BlockingReader = MadsimReader;
+    type Writer = MadsimWriter;
+    type BlockingWriter = MadsimWriter;
+    type Pager = MadsimPager;
+    type BlockingPager = MadsimPager;
+
+    fn inner(&self) -> &Self::Inner {
+        &()
+    }
+
+    async fn read(&self, path: &str, args: OpRead) -> crate::Result<(RpRead, 
Self::Reader)> {
+        let handle = self.handle.inner.lock().unwrap();
+        let length = handle
+            .get(path)
+            .map(|(_, data)| data.len())
+            .unwrap_or_default();
+        Ok((
+            RpRead::new(length as u64),
+            MadsimReader {
+                path: path.to_string(),
+                args,
+                handle: self.handle.clone(),
+            },
+        ))
+    }
+
+    async fn write(&self, path: &str, args: OpWrite) -> 
crate::Result<(RpWrite, Self::Writer)> {
+        Ok((
+            RpWrite::new(),
+            MadsimWriter {
+                path: path.to_string(),
+                args,
+                handle: self.handle.clone(),
+            },
+        ))
+    }
+
+    async fn list(&self, path: &str, args: OpList) -> crate::Result<(RpList, 
Self::Pager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    async fn scan(&self, path: &str, args: OpScan) -> crate::Result<(RpScan, 
Self::Pager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn blocking_read(
+        &self,
+        path: &str,
+        args: OpRead,
+    ) -> crate::Result<(RpRead, Self::BlockingReader)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }

Review Comment:
   Generally we don't support running a blocking function in the simulation, as 
it will block the only execution thread. In other words, all operations which 
can not complete immediately should be defined as async functions.
   So these blocking-prefixed functions should be either unsupported at all, or 
be considered happen at an instant.



##########
core/src/layers/madsim.rs:
##########


Review Comment:
   Overall the current implementation looks identical to the FS simulator in 
madsim. Will the follow-up still be the same? I'm not sure if it is the right 
direction. Because in FS, data is node-local and can't be accessed by other 
nodes. Is that align with the semantics of OpenDAL services?



##########
core/src/layers/madsim.rs:
##########
@@ -0,0 +1,316 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ops::{OpList, OpRead, OpScan, OpWrite};
+use crate::raw::oio::Entry;
+use crate::raw::{oio, Accessor, Layer, LayeredAccessor, RpList, RpRead, 
RpScan, RpWrite};
+use crate::{EntryMode, Metadata};
+use async_trait::async_trait;
+use bytes::Bytes;
+use madsim::plugin::simulator;
+use madsim::plugin::Simulator;
+use madsim::rand::GlobalRng;
+use madsim::runtime::Handle;
+use madsim::task::NodeId;
+use madsim::time::TimeHandle;
+use madsim::Config;
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::io::SeekFrom;
+use std::sync::{Arc, Mutex};
+use std::task::{Context, Poll};
+
+#[derive(Debug, Copy, Clone)]
+pub struct MadsimLayer {
+    service_sim_node: NodeId,
+}
+
+impl MadsimLayer {
+    pub fn new() -> Self {
+        let service_sim_node = Handle::current().create_node().build().id();
+        Self { service_sim_node }
+    }
+}
+
+impl<A: Accessor> Layer<A> for MadsimLayer {
+    type LayeredAccessor = MadsimAccessor;
+
+    fn layer(&self, _inner: A) -> Self::LayeredAccessor {
+        let handle = simulator::<ServiceSim>().get_node(self.service_sim_node);
+        MadsimAccessor { handle }
+    }
+}
+
+#[derive(Debug)]
+pub struct MadsimAccessor {
+    handle: ServiceNodeHandle,
+}
+
+#[async_trait]
+impl LayeredAccessor for MadsimAccessor {
+    type Inner = ();
+    type Reader = MadsimReader;
+    type BlockingReader = MadsimReader;
+    type Writer = MadsimWriter;
+    type BlockingWriter = MadsimWriter;
+    type Pager = MadsimPager;
+    type BlockingPager = MadsimPager;
+
+    fn inner(&self) -> &Self::Inner {
+        &()
+    }
+
+    async fn read(&self, path: &str, args: OpRead) -> crate::Result<(RpRead, 
Self::Reader)> {
+        let handle = self.handle.inner.lock().unwrap();
+        let length = handle
+            .get(path)
+            .map(|(_, data)| data.len())
+            .unwrap_or_default();
+        Ok((
+            RpRead::new(length as u64),
+            MadsimReader {
+                path: path.to_string(),
+                args,
+                handle: self.handle.clone(),
+            },
+        ))
+    }
+
+    async fn write(&self, path: &str, args: OpWrite) -> 
crate::Result<(RpWrite, Self::Writer)> {
+        Ok((
+            RpWrite::new(),
+            MadsimWriter {
+                path: path.to_string(),
+                args,
+                handle: self.handle.clone(),
+            },
+        ))
+    }
+
+    async fn list(&self, path: &str, args: OpList) -> crate::Result<(RpList, 
Self::Pager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    async fn scan(&self, path: &str, args: OpScan) -> crate::Result<(RpScan, 
Self::Pager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn blocking_read(
+        &self,
+        path: &str,
+        args: OpRead,
+    ) -> crate::Result<(RpRead, Self::BlockingReader)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn blocking_write(
+        &self,
+        path: &str,
+        args: OpWrite,
+    ) -> crate::Result<(RpWrite, Self::BlockingWriter)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn blocking_list(
+        &self,
+        path: &str,
+        args: OpList,
+    ) -> crate::Result<(RpList, Self::BlockingPager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn blocking_scan(
+        &self,
+        path: &str,
+        args: OpScan,
+    ) -> crate::Result<(RpScan, Self::BlockingPager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+pub struct MadsimReader {
+    path: String,
+    args: OpRead,
+    handle: ServiceNodeHandle,
+}
+
+impl oio::Read for MadsimReader {
+    fn poll_read(&mut self, _cx: &mut Context<'_>, buf: &mut [u8]) -> 
Poll<crate::Result<usize>> {
+        let mut inner = self.handle.inner.lock().unwrap();
+        if let Some((_meta, data)) = inner.get(&self.path) {
+            buf.copy_from_slice(data);
+            Poll::Ready(Ok(data.len()))
+        } else {
+            Poll::Ready(Err(crate::Error::new(
+                crate::ErrorKind::NotFound,
+                "not found",
+            )))
+        }
+    }
+
+    fn poll_seek(&mut self, cx: &mut Context<'_>, pos: SeekFrom) -> 
Poll<crate::Result<u64>> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn poll_next(&mut self, cx: &mut Context<'_>) -> 
Poll<Option<crate::Result<Bytes>>> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+impl oio::BlockingRead for MadsimReader {
+    fn read(&mut self, buf: &mut [u8]) -> crate::Result<usize> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn seek(&mut self, pos: SeekFrom) -> crate::Result<u64> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn next(&mut self) -> Option<crate::Result<Bytes>> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+pub struct MadsimWriter {
+    path: String,
+    args: OpWrite,
+    handle: ServiceNodeHandle,
+}
+
+impl oio::BlockingWrite for MadsimWriter {
+    fn write(&mut self, bs: Bytes) -> crate::Result<()> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn append(&mut self, bs: Bytes) -> crate::Result<()> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn close(&mut self) -> crate::Result<()> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+#[async_trait]
+impl oio::Write for MadsimWriter {
+    async fn write(&mut self, bs: Bytes) -> crate::Result<()> {
+        self.handle.write(&self.path, &self.args, bs);
+        Ok(())
+    }
+
+    async fn append(&mut self, bs: Bytes) -> crate::Result<()> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    async fn abort(&mut self) -> crate::Result<()> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    async fn close(&mut self) -> crate::Result<()> {
+        Ok(())
+    }
+}
+
+pub struct MadsimPager {}
+
+#[async_trait]
+impl oio::Page for MadsimPager {
+    async fn next(&mut self) -> crate::Result<Option<Vec<Entry>>> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+impl oio::BlockingPage for MadsimPager {
+    fn next(&mut self) -> crate::Result<Option<Vec<Entry>>> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+#[derive(Default)]
+pub struct ServiceSim {
+    handles: Mutex<HashMap<NodeId, ServiceNodeHandle>>,
+}
+
+impl Simulator for ServiceSim {
+    fn new(_rand: &GlobalRng, _time: &TimeHandle, _config: &Config) -> Self
+    where
+        Self: Sized,
+    {
+        Self::default()
+    }
+    fn create_node(&self, id: NodeId) {
+        let mut handles = self.handles.lock().unwrap();
+        handles.insert(id, ServiceNodeHandle::default());
+    }
+    fn reset_node(&self, _id: NodeId) {}

Review Comment:
   Don't forget to implement this function. Perhaps inserting a new 
`ServiceNodeHandle` to replace the old one.



##########
core/src/layers/madsim.rs:
##########
@@ -0,0 +1,316 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::ops::{OpList, OpRead, OpScan, OpWrite};
+use crate::raw::oio::Entry;
+use crate::raw::{oio, Accessor, Layer, LayeredAccessor, RpList, RpRead, 
RpScan, RpWrite};
+use crate::{EntryMode, Metadata};
+use async_trait::async_trait;
+use bytes::Bytes;
+use madsim::plugin::simulator;
+use madsim::plugin::Simulator;
+use madsim::rand::GlobalRng;
+use madsim::runtime::Handle;
+use madsim::task::NodeId;
+use madsim::time::TimeHandle;
+use madsim::Config;
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::io::SeekFrom;
+use std::sync::{Arc, Mutex};
+use std::task::{Context, Poll};
+
+#[derive(Debug, Copy, Clone)]
+pub struct MadsimLayer {
+    service_sim_node: NodeId,
+}
+
+impl MadsimLayer {
+    pub fn new() -> Self {
+        let service_sim_node = Handle::current().create_node().build().id();
+        Self { service_sim_node }
+    }
+}
+
+impl<A: Accessor> Layer<A> for MadsimLayer {
+    type LayeredAccessor = MadsimAccessor;
+
+    fn layer(&self, _inner: A) -> Self::LayeredAccessor {
+        let handle = simulator::<ServiceSim>().get_node(self.service_sim_node);
+        MadsimAccessor { handle }
+    }
+}
+
+#[derive(Debug)]
+pub struct MadsimAccessor {
+    handle: ServiceNodeHandle,
+}
+
+#[async_trait]
+impl LayeredAccessor for MadsimAccessor {
+    type Inner = ();
+    type Reader = MadsimReader;
+    type BlockingReader = MadsimReader;
+    type Writer = MadsimWriter;
+    type BlockingWriter = MadsimWriter;
+    type Pager = MadsimPager;
+    type BlockingPager = MadsimPager;
+
+    fn inner(&self) -> &Self::Inner {
+        &()
+    }
+
+    async fn read(&self, path: &str, args: OpRead) -> crate::Result<(RpRead, 
Self::Reader)> {
+        let handle = self.handle.inner.lock().unwrap();
+        let length = handle
+            .get(path)
+            .map(|(_, data)| data.len())
+            .unwrap_or_default();
+        Ok((
+            RpRead::new(length as u64),
+            MadsimReader {
+                path: path.to_string(),
+                args,
+                handle: self.handle.clone(),
+            },
+        ))
+    }
+
+    async fn write(&self, path: &str, args: OpWrite) -> 
crate::Result<(RpWrite, Self::Writer)> {
+        Ok((
+            RpWrite::new(),
+            MadsimWriter {
+                path: path.to_string(),
+                args,
+                handle: self.handle.clone(),
+            },
+        ))
+    }
+
+    async fn list(&self, path: &str, args: OpList) -> crate::Result<(RpList, 
Self::Pager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    async fn scan(&self, path: &str, args: OpScan) -> crate::Result<(RpScan, 
Self::Pager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn blocking_read(
+        &self,
+        path: &str,
+        args: OpRead,
+    ) -> crate::Result<(RpRead, Self::BlockingReader)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn blocking_write(
+        &self,
+        path: &str,
+        args: OpWrite,
+    ) -> crate::Result<(RpWrite, Self::BlockingWriter)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn blocking_list(
+        &self,
+        path: &str,
+        args: OpList,
+    ) -> crate::Result<(RpList, Self::BlockingPager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn blocking_scan(
+        &self,
+        path: &str,
+        args: OpScan,
+    ) -> crate::Result<(RpScan, Self::BlockingPager)> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+pub struct MadsimReader {
+    path: String,
+    args: OpRead,
+    handle: ServiceNodeHandle,
+}
+
+impl oio::Read for MadsimReader {
+    fn poll_read(&mut self, _cx: &mut Context<'_>, buf: &mut [u8]) -> 
Poll<crate::Result<usize>> {
+        let mut inner = self.handle.inner.lock().unwrap();
+        if let Some((_meta, data)) = inner.get(&self.path) {
+            buf.copy_from_slice(data);
+            Poll::Ready(Ok(data.len()))
+        } else {
+            Poll::Ready(Err(crate::Error::new(
+                crate::ErrorKind::NotFound,
+                "not found",
+            )))
+        }
+    }
+
+    fn poll_seek(&mut self, cx: &mut Context<'_>, pos: SeekFrom) -> 
Poll<crate::Result<u64>> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn poll_next(&mut self, cx: &mut Context<'_>) -> 
Poll<Option<crate::Result<Bytes>>> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+impl oio::BlockingRead for MadsimReader {
+    fn read(&mut self, buf: &mut [u8]) -> crate::Result<usize> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn seek(&mut self, pos: SeekFrom) -> crate::Result<u64> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn next(&mut self) -> Option<crate::Result<Bytes>> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+pub struct MadsimWriter {
+    path: String,
+    args: OpWrite,
+    handle: ServiceNodeHandle,
+}
+
+impl oio::BlockingWrite for MadsimWriter {
+    fn write(&mut self, bs: Bytes) -> crate::Result<()> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn append(&mut self, bs: Bytes) -> crate::Result<()> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+
+    fn close(&mut self) -> crate::Result<()> {
+        unimplemented!("currently not supported, will be implemented in the 
future")
+    }
+}
+
+#[async_trait]
+impl oio::Write for MadsimWriter {

Review Comment:
   Just curious why the `Write` trait is async-trait style while the `Read` 
trait is poll style. 😄 @Xuanwo  



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to