zuston commented on code in PR #1311:
URL:
https://github.com/apache/incubator-uniffle/pull/1311#discussion_r1397084825
##########
rust/experimental/server/src/store/memory.rs:
##########
@@ -413,6 +413,18 @@ impl Store for MemoryStore {
last_block_id = -1;
}
}
+
+ // get block_ids filter
+ // In AQE, after executing the sub-QueryStages,
collect the shuffle data size
+ // So if we can filter block, it will improve the
performance of AQE.
+ candidate_blocks = candidate_blocks
Review Comment:
This part looks not correct. I hope this could be bound to the
`read_partial_data_with_max_size_limit` .
Without the bitmap filter, we will read it the (last_block_id, end] without
the limited size. But for now, I think it will read from (last_block_id, end]
with filter and then in a limited size. Right?
So it's better to refactor the `read_partial_data_with_max_size_limit`,
maybe the name of `read_partial_data_with_max_size_limit_and_filter` will be
better. Like that
```
pub(crate) fn read_partial_data_with_max_size_limit_and_task_filter<'a>(
&'a self,
blocks: Vec<&'a PartitionedDataBlock>,
fetched_size_limit: i64,
serialized_expected_task_ids_bitmap: Option<Treemap>,
) -> (Vec<&PartitionedDataBlock>, i64) {
let mut fetched = vec![];
let mut fetched_size = 0;
for block in blocks {
if &serialized_expected_task_ids_bitmap.is_some() {
if
!&serialized_expected_task_ids_bitmap.unwrap().contains(block.task_attempt_id
as u64) {
continue;
}
}
if fetched_size >= fetched_size_limit {
break;
}
fetched_size += block.length as i64;
fetched.push(block);
}
(fetched, fetched_size)
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
