-
Notifications
You must be signed in to change notification settings - Fork 333
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: support fetch ranges in concurrent #2959
feat: support fetch ranges in concurrent #2959
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2959 +/- ##
===========================================
- Coverage 85.90% 85.39% -0.51%
===========================================
Files 780 780
Lines 125900 125980 +80
===========================================
- Hits 108155 107585 -570
- Misses 17745 18395 +650 |
d90146c
to
a70eebf
Compare
A great job! But I think we can do a benchmark for this feature. We don't know if this feature brings the performance upgrading or downgrading. |
@QuenKar Could you please add the benchmark result to the description? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
a70eebf
to
a6265e4
Compare
Similar idea: apache/opendal#3675 |
I hereby agree to the terms of the GreptimeDB CLA
What's changed and what's your intention?
support concurrently fetch page ranges.
now the policy is that if object store support blocking, use blocking read priorly,
if not, use concurrently read.
Benchmark
Env
Local PC: mac m2 16G RAM 512 SSD
Config
[storage]
type = S3
region = minipc-0
root = "test-concurrent-read"
bucket = "test-bucket-0"
Result
fetch parquet according to ranges in sequence use develop branch, and this branch is concurrent.
conclusion
It's still slightly improved until
lastpoint query
,because the
lastpoint query
basically reads the whole table, and all the pages are cached in memory, and there is not much difference in the following queries.Checklist
Refer to a related PR or issue link (optional)
Closes #2808