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

Allow readers to read consistently #1043

Merged
merged 2 commits into from
Oct 28, 2024
Merged

Conversation

blt
Copy link
Collaborator

@blt blt commented Oct 17, 2024

What does this PR do?

This commit exposes a read_at on the block cache allowing callers to
read the same data from offset X over and over again. This means that
when we expose data from a block cache in the logrotate filesystem we
are able to correctly do wc -l and similar.

@blt blt added the no-changelog label Oct 17, 2024 — with Graphite App
@blt blt marked this pull request as ready for review October 17, 2024 00:39
@blt blt requested a review from a team as a code owner October 17, 2024 00:39
@blt blt force-pushed the blt/ensure_file_size_updates_due_to_getattr branch from 8f3227c to 3cfa40a Compare October 22, 2024 23:50
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from 06b8ee6 to d165859 Compare October 22, 2024 23:51
This was referenced Oct 22, 2024
@blt blt force-pushed the blt/ensure_file_size_updates_due_to_getattr branch from 3cfa40a to 61386a0 Compare October 23, 2024 23:54
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from d165859 to 7e4aed0 Compare October 23, 2024 23:54
@blt blt force-pushed the blt/ensure_file_size_updates_due_to_getattr branch from 61386a0 to 8f3f944 Compare October 23, 2024 23:58
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from 7e4aed0 to 54cf3ea Compare October 23, 2024 23:58
@blt blt force-pushed the blt/ensure_file_size_updates_due_to_getattr branch from 8f3f944 to 854d58d Compare October 24, 2024 00:02
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from 54cf3ea to ea176e5 Compare October 24, 2024 00:02
@blt blt force-pushed the blt/ensure_file_size_updates_due_to_getattr branch from 854d58d to 340087f Compare October 24, 2024 00:10
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from ea176e5 to 207dd72 Compare October 24, 2024 00:10
@blt blt force-pushed the blt/ensure_file_size_updates_due_to_getattr branch from 340087f to 1d0b1f0 Compare October 24, 2024 00:20
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from 207dd72 to a03cff4 Compare October 24, 2024 00:20
@blt blt force-pushed the blt/ensure_file_size_updates_due_to_getattr branch from 1d0b1f0 to 8c02a22 Compare October 24, 2024 00:26
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from a03cff4 to 5424a84 Compare October 24, 2024 00:26
This was referenced Oct 25, 2024
@blt blt force-pushed the blt/ensure_file_size_updates_due_to_getattr branch from 191bcb0 to 55b85e2 Compare October 28, 2024 15:47
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from 19e5b4f to d62fce9 Compare October 28, 2024 15:48
Comment on lines +417 to +441
while remaining > 0 {
// The plan is this. We treat the blocks as one infinite cycle. We
// map our offset into the domain of the blocks, then seek forward
// until we find the block we need to start reading from. Then we
// read into `data`.

let offset_within_cycle = current_offset % total_cycle_size;
let mut block_start = 0;
for block in blocks {
let block_size = block.total_bytes.get() as usize;
if offset_within_cycle < block_start + block_size {
// Offset is within this block. Begin reading into `data`.
let block_offset = offset_within_cycle - block_start;
let bytes_in_block = (block_size - block_offset).min(remaining);

data.extend_from_slice(
&block.bytes[block_offset..block_offset + bytes_in_block],
);

remaining -= bytes_in_block;
current_offset += bytes_in_block;
break;
}
block_start += block_size;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to stare at this loop for a while to appreciate the subtleties involved (reads crossing block boundaries, cycle boundaries). Neat stuff.

@blt blt force-pushed the blt/ensure_file_size_updates_due_to_getattr branch 2 times, most recently from b5e67d9 to 4206504 Compare October 28, 2024 16:26
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch 2 times, most recently from 0e316da to 4fa0e24 Compare October 28, 2024 16:29
@blt blt changed the base branch from blt/ensure_file_size_updates_due_to_getattr to graphite-base/1043 October 28, 2024 17:15
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from 4fa0e24 to 68a0760 Compare October 28, 2024 17:15
@blt blt changed the base branch from graphite-base/1043 to main October 28, 2024 17:16
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from 68a0760 to 5501775 Compare October 28, 2024 17:16
This commit exposes a `read_at` on the block cache allowing callers to
read the same data from offset X over and over again. This means that
when we expose data from a block cache in the logrotate filesystem we
are able to correctly do `wc -l` and similar.

Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
@blt blt force-pushed the blt/allow_readers_to_read_consistently branch from 5501775 to ffae4d1 Compare October 28, 2024 17:46
@blt blt merged commit e619a9e into main Oct 28, 2024
16 checks passed
Copy link
Collaborator Author

blt commented Oct 28, 2024

Merge activity

  • Oct 28, 2:23 PM EDT: A user merged this pull request with Graphite.

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

Successfully merging this pull request may close these issues.

2 participants