diqiu50 commented on code in PR #5905:
URL: https://github.com/apache/gravitino/pull/5905#discussion_r1893344497
##########
clients/filesystem-fuse/src/fuse_api_handle.rs:
##########
@@ -30,10 +30,174 @@ use fuse3::{Errno, FileType, Inode, SetAttr, Timestamp};
use futures_util::stream;
use futures_util::stream::BoxStream;
use futures_util::StreamExt;
+use log::debug;
use std::ffi::{OsStr, OsString};
use std::num::NonZeroU32;
use std::time::{Duration, SystemTime};
+pub(crate) struct FuseApiHandleDebug<T: RawFileSystem> {
+ inner: FuseApiHandle<T>,
+}
+
+impl<T: RawFileSystem> FuseApiHandleDebug<T> {
+ pub fn new(fs: T, context: FileSystemContext) -> Self {
+ Self {
+ inner: FuseApiHandle::new(fs, context),
+ }
+ }
+
+ pub async fn get_file_path(&self, file_id: u64) -> String {
+ debug!("get_file_path: file_id: {}", file_id);
+ let result = self.inner.get_file_path(file_id).await;
+ debug!("get_file_path result: {}", result);
+ result
+ }
+
+ async fn get_modified_file_stat(
+ &self,
+ file_id: u64,
+ size: Option<u64>,
+ atime: Option<Timestamp>,
+ mtime: Option<Timestamp>,
+ ) -> Result<FileStat, Errno> {
+ debug!("get_modified_file_stat: file_id: {}, size: {:?}, atime: {:?},
mtime: {:?}", file_id, size, atime, mtime);
+ let result = self.inner.get_modified_file_stat(file_id, size, atime,
mtime).await;
+ debug!("get_modified_file_stat result: {:?}", result);
+ result
+ }
+}
+
+impl<T: RawFileSystem> Filesystem for FuseApiHandleDebug<T> {
+ async fn init(&self, req: Request) -> fuse3::Result<ReplyInit> {
+ debug!("init: req: {:?}", req);
+ let result = self.inner.init(req).await;
+ debug!("init result: {:?}", result);
Review Comment:
This is an asynchronous multithreaded model. Logs from two print statements
within a single function may be interleaved with logs from other interfaces. It
is necessary to identify them in the logs.
##########
clients/filesystem-fuse/src/fuse_api_handle.rs:
##########
@@ -30,10 +30,174 @@ use fuse3::{Errno, FileType, Inode, SetAttr, Timestamp};
use futures_util::stream;
use futures_util::stream::BoxStream;
use futures_util::StreamExt;
+use log::debug;
use std::ffi::{OsStr, OsString};
use std::num::NonZeroU32;
use std::time::{Duration, SystemTime};
+pub(crate) struct FuseApiHandleDebug<T: RawFileSystem> {
+ inner: FuseApiHandle<T>,
+}
+
+impl<T: RawFileSystem> FuseApiHandleDebug<T> {
+ pub fn new(fs: T, context: FileSystemContext) -> Self {
+ Self {
+ inner: FuseApiHandle::new(fs, context),
+ }
+ }
+
Review Comment:
These functions that are not part of the Filesystem trait do not need to add
debug logs.
##########
clients/filesystem-fuse/src/fuse_api_handle.rs:
##########
@@ -30,10 +30,174 @@ use fuse3::{Errno, FileType, Inode, SetAttr, Timestamp};
use futures_util::stream;
use futures_util::stream::BoxStream;
use futures_util::StreamExt;
+use log::debug;
use std::ffi::{OsStr, OsString};
use std::num::NonZeroU32;
use std::time::{Duration, SystemTime};
+pub(crate) struct FuseApiHandleDebug<T: RawFileSystem> {
+ inner: FuseApiHandle<T>,
+}
+
+impl<T: RawFileSystem> FuseApiHandleDebug<T> {
+ pub fn new(fs: T, context: FileSystemContext) -> Self {
+ Self {
+ inner: FuseApiHandle::new(fs, context),
+ }
+ }
+
+ pub async fn get_file_path(&self, file_id: u64) -> String {
+ debug!("get_file_path: file_id: {}", file_id);
+ let result = self.inner.get_file_path(file_id).await;
+ debug!("get_file_path result: {}", result);
+ result
+ }
+
+ async fn get_modified_file_stat(
+ &self,
+ file_id: u64,
+ size: Option<u64>,
+ atime: Option<Timestamp>,
+ mtime: Option<Timestamp>,
+ ) -> Result<FileStat, Errno> {
+ debug!("get_modified_file_stat: file_id: {}, size: {:?}, atime: {:?},
mtime: {:?}", file_id, size, atime, mtime);
+ let result = self.inner.get_modified_file_stat(file_id, size, atime,
mtime).await;
+ debug!("get_modified_file_stat result: {:?}", result);
+ result
+ }
+}
+
+impl<T: RawFileSystem> Filesystem for FuseApiHandleDebug<T> {
+ async fn init(&self, req: Request) -> fuse3::Result<ReplyInit> {
+ debug!("init: req: {:?}", req);
+ let result = self.inner.init(req).await;
+ debug!("init result: {:?}", result);
+ result
+ }
+
+ async fn destroy(&self, req: Request) {
+ debug!("destroy: req: {:?}", req);
+ self.inner.destroy(req).await;
+ debug!("destroy completed");
+ }
+
+ async fn lookup(&self, req: Request, parent: Inode, name: &OsStr) ->
fuse3::Result<ReplyEntry> {
+ debug!("lookup: req: {:?}, parent: {:?}, name: {:?}", req, parent,
name);
+ let result = self.inner.lookup(req, parent, name).await;
+ debug!("lookup result: {:?}", result);
Review Comment:
If an error is returned, it should be logged using error!.
##########
clients/filesystem-fuse/src/fuse_api_handle.rs:
##########
@@ -30,10 +30,174 @@ use fuse3::{Errno, FileType, Inode, SetAttr, Timestamp};
use futures_util::stream;
use futures_util::stream::BoxStream;
use futures_util::StreamExt;
+use log::debug;
use std::ffi::{OsStr, OsString};
use std::num::NonZeroU32;
use std::time::{Duration, SystemTime};
+pub(crate) struct FuseApiHandleDebug<T: RawFileSystem> {
+ inner: FuseApiHandle<T>,
+}
+
+impl<T: RawFileSystem> FuseApiHandleDebug<T> {
+ pub fn new(fs: T, context: FileSystemContext) -> Self {
+ Self {
+ inner: FuseApiHandle::new(fs, context),
+ }
+ }
+
+ pub async fn get_file_path(&self, file_id: u64) -> String {
+ debug!("get_file_path: file_id: {}", file_id);
+ let result = self.inner.get_file_path(file_id).await;
+ debug!("get_file_path result: {}", result);
+ result
+ }
+
+ async fn get_modified_file_stat(
+ &self,
+ file_id: u64,
+ size: Option<u64>,
+ atime: Option<Timestamp>,
+ mtime: Option<Timestamp>,
+ ) -> Result<FileStat, Errno> {
+ debug!("get_modified_file_stat: file_id: {}, size: {:?}, atime: {:?},
mtime: {:?}", file_id, size, atime, mtime);
+ let result = self.inner.get_modified_file_stat(file_id, size, atime,
mtime).await;
+ debug!("get_modified_file_stat result: {:?}", result);
+ result
+ }
+}
+
+impl<T: RawFileSystem> Filesystem for FuseApiHandleDebug<T> {
+ async fn init(&self, req: Request) -> fuse3::Result<ReplyInit> {
Review Comment:
We need to implement all interfaces of the Filesystem. If they are not
implemented yet, an error log should be recorded.
--
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: commits-unsubscr...@gravitino.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
