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

[pixels-presto] support concurrent read. #172

Closed
bianhq opened this issue Feb 16, 2022 · 1 comment
Closed

[pixels-presto] support concurrent read. #172

bianhq opened this issue Feb 16, 2022 · 1 comment
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@bianhq
Copy link
Contributor

bianhq commented Feb 16, 2022

PageSource is used to get pages concurrently in Presto, so the Page objects generated by PageSource should not share the same set of column blocks. Otherwise, the consecutively generated Pages will be overwritten and thus have the same content.

However, using different blocks for pages will introduce more GC overhead. We try to support concurrent read without increasing the GC burden in this issue. A possible solution is to use the RecordCursor.

@bianhq bianhq added bug Something isn't working enhancement New feature or request labels Feb 16, 2022
@bianhq bianhq added this to the GC Optimization milestone Feb 16, 2022
@bianhq bianhq self-assigned this Feb 16, 2022
bianhq added a commit that referenced this issue Feb 19, 2022
1. Add the PixelsRecordSet and PixelsRecordCusor, which can be enabled/disabled by configuration properties, to pixels-presto.
2. Update PixelsRecordReaderImpl.readBatch, let users specify whether they want to reuse the returned result row batch.

Issue #172 is not completely resolved. Record cursor is much slower than the page source for wide tables (they have similar performance on TPC-H). We also tried to implement a context (buffer) of result row batches to automatically allocate, free, and reuse the result row batches. However, it reduces the query performance a lot. This issue can be solved by implementing the read path in C++ in the future.
@bianhq
Copy link
Contributor Author

bianhq commented Feb 19, 2022

Concurrent read is supported, but GC overhead is still there.

@bianhq bianhq closed this as completed Feb 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant