Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(services/monoiofs): impl read and write, add behavior test #4944

Merged
merged 10 commits into from
Aug 21, 2024

Conversation

NKID00
Copy link
Contributor

@NKID00 NKID00 commented Jul 30, 2024

Part of #4552.

This PR implements basic read and write operations for monoiofs. The implementation differs from compfs in that file opened with monoio is stored within the worker thread because it is non-Send and non-Sync. Read and write operations send buffers to worker thread and receive results instead of send futures.

This PR also introduces behavior tests for monoiofs. Implementations of stat and delete are borrowed from FsBackend to satisfy requirements of behavior tests. They are not implemented natively because monoio's support for file metadata is not released yet and file deletion is currently not implemented.

@NKID00
Copy link
Contributor Author

NKID00 commented Jul 30, 2024

There seems to be an unknown bug (probably some race condition) that causes monoio runtime to occasionally fail to initialize (seems only affect Node.js binding). I'll take a deeper look.

@Xuanwo
Copy link
Member

Xuanwo commented Jul 31, 2024

Thanks for your PR! I will review it later this week.

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM except the monoio part. We can work together at the upstream.

core/src/services/monoiofs/backend.rs Outdated Show resolved Hide resolved
core/src/services/monoiofs/backend.rs Outdated Show resolved Hide resolved
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@Xuanwo Xuanwo merged commit 90832a4 into apache:main Aug 21, 2024
221 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants