Group operations for maybe_get_required_blobs
#4938
Conversation
afck
left a comment
afck
left a comment
There was a problem hiding this comment.
Looks good to me; although it does make the code more complicated and in practice probably only a small fraction of blocks have a significant number of blobs. Either way, would be good to add some more comments to the code to make it easier to read.
| *maybe_blob = Some(blob); | ||
| break; | ||
| } | ||
| for (_, pending_blobs) in self |
There was a problem hiding this comment.
You probably want to continue here if maybe_blob.is_some()?
There was a problem hiding this comment.
Yes, you are very much correct.
I changed that part of the code significantly, though.
| for (index, blob_id) in missing_indices.into_iter().zip(missing_blob_ids) { | ||
| for (_, pending_blob) in &pending_blobs { | ||
| if let Some(blob) = pending_blob.get(&blob_id).await? { | ||
| maybe_blobs[index].1 = Some(blob); |
| break; | ||
| .await?; | ||
| for (index, blob_id) in missing_indices.into_iter().zip(missing_blob_ids) { | ||
| for (_, pending_blob) in &pending_blobs { |
There was a problem hiding this comment.
(Strictly speaking, it's not a single pending_blob. It's still a collection of pending blobs that belong to a single block.)
There was a problem hiding this comment.
Ok. renamed "pending_blobs" as "all_entries_pending_blobs" and "pending_blob" as "pending_blobs".
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
2 participants
Motivation
The
pending_proposed_blobsneeds to be simplified. But before that, we can restructure the access code.Proposal
Group the operations in the querying of the data in
maybe_get_required_blobs.Test Plan
The CI.
Release Plan
Links
None.