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

Add rust extension lib alluxiocommon to the repo #39

Merged
merged 34 commits into from
Apr 29, 2024

Conversation

lucyge2022
Copy link
Collaborator

Add rust extension lib alluxiocommon to the repo, it currently only handles:
spawn on demand or preallocated threadpool to make multiple http getpage request concurrently to fetch pages

this extension can be enabled or disabled via options in AlluxioFileSystem as well as in AlluxioClient

…hread

5f80059 small fixes for REST and metrics
1c426c2 add access debug logs for all fsspec ops & add quick sanity unit tests for read
7dcb833 checkin multi process http benchmark for worker http server
4b4ebce use alluxiocommon pyo3 package to try to do multithread http req
523f748 add rust pyo3 extension
8979d2b remove first
0367659 add related files
4d0eebe small fixes
a0519b3 if any error on any thread running result, raise exception back to python directly
…_alluxio_fsspec.py

2. make tests/client/test_read_range_docker.py (now named as test_read_docker.py
to handle full read as well
add alluxiocommon fixture in it too
Copy link
Collaborator

@LuQQiu LuQQiu left a comment

Choose a reason for hiding this comment

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

LGTM

@lucyge2022 lucyge2022 force-pushed the rust-extension branch 2 times, most recently from a448110 to 647e3ea Compare April 26, 2024 22:30
Copy link
Collaborator

@jja725 jja725 left a comment

Choose a reason for hiding this comment

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

Overall LGTM, Thanks for the effort!


[dependencies]
reqwest = { version = "0.11", features = ["blocking"] }
tokio = { version = "1", features = ["full"] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

full tokio is a big stuff, probably we only need a specific part of it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

understood, will check the related pieces to check in

}

for sender in senders {
let result = sender.unwrap().blocking_recv();
Copy link
Collaborator

@jja725 jja725 Apr 26, 2024

Choose a reason for hiding this comment

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

I feel like this could be simplified with rayon::iter::ParallelIterator.try_for_each. We just generate a iterate of pageid and use ParallelIterator, the output is ordered so in that case we can get rid of the oneshot channel as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah u r right i will try that, i plan to clean up this lib after i check this extension it, will address this in later PR

@lucyge2022 lucyge2022 added the enhancement New feature or request label Apr 29, 2024
@lucyge2022
Copy link
Collaborator Author

alluxio-bot, merge this please.

@lucyge2022 lucyge2022 merged commit 08429c4 into fsspec:main Apr 29, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants