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

Make get_page_stream "spawn-friendly"? #61

Closed
dantengsky opened this issue Oct 14, 2021 · 1 comment · Fixed by #62
Closed

Make get_page_stream "spawn-friendly"? #61

dantengsky opened this issue Oct 14, 2021 · 1 comment · Fixed by #62

Comments

@dantengsky
Copy link
Contributor

For code like tokio::spawn(get_page_stream(col_chunk_meta, reader, buffer, page_filter)), which requires the Future being spawned should be Send, rustc complains about the parameter page_filter as follows:

type Arc<dyn for<'r, 's> Fn(&'r ColumnDescriptor, &'s DataPageHeader) -> bool> which is not Send


Currently the signature of get_page_stream and the type def of PageFilter are:

pub async fn get_page_stream<'a, RR: AsyncRead + Unpin + Send + AsyncSeek>(
column_metadata: &'a ColumnChunkMetaData,
reader: &'a mut RR,
buffer: Vec<u8>,
pages_filter: PageFilter,
) -> Result<impl Stream<Item = Result<CompressedDataPage>> + 'a> {

pub type PageFilter = Arc<dyn Fn(&ColumnDescriptor, &DataPageHeader) -> bool>;


  • Should other APIs be used to get pages asynchronously in this case?

  • Is it possible/proper to change the type of PageFilter to

    pub type PageFilter = Arc<dyn Fn(&ColumnDescriptor, &DataPageHeader) -> bool + Send + Sync>; 

    so that PageFilter will be Send

@jorgecarleitao
Copy link
Owner

Great observation. Thanks for it and for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants