Skip to content

Eviction/chunk lru#27

Open
2a46m4 wants to merge 9 commits intomainfrom
eviction/chunk-lru
Open

Eviction/chunk lru#27
2a46m4 wants to merge 9 commits intomainfrom
eviction/chunk-lru

Conversation

@2a46m4
Copy link
Copy Markdown
Collaborator

@2a46m4 2a46m4 commented Mar 21, 2025

No description provided.

@johnramsden
Copy link
Copy Markdown
Owner

Right now I'm hitting an assertion:

assert(res->value.location.id == data_id);

@johnramsden johnramsden self-requested a review April 2, 2025 02:11
assert(buffer);

long write_ptr = CHUNK_POINTER(cache->zone_cap, cache->chunk_sz, 0, zone_id);
size_t bytes = pread(cache->fd, buffer, cache->zone_cap, write_ptr);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't think this is sufficient. We need to do something like this

ZNCache/src/zncache.c

Lines 211 to 237 in 04a939c

read_workload(int fd, uint32_t *buffer, size_t size) {
size_t total_bytes_read = 0;
while (total_bytes_read < size) {
errno = 0;
ssize_t bytes_read = read(fd, buffer + total_bytes_read, size - total_bytes_read);
if (bytes_read < 0) {
if (errno == EINTR) {
// Interrupted
continue;
}
fprintf(stderr, "Couldn't read the file: '%s'\n", strerror(errno));
return 1;
}
if (bytes_read == 0) {
break;
}
total_bytes_read += bytes_read;
}
if (total_bytes_read != size) {
fprintf(stderr, "Couldn't read the file fully, read %zu bytes out of %zu\n", total_bytes_read, size);
return 1;
}
return 0;
}
. It's not guaranteed to always return all of the bytes and you might need to do further calls

src/cachemap.c Outdated
assert(res);
assert(res->type == RESULT_LOC);
assert(res->value.location.chunk_offset == chunk_offset);
assert(res->value.location.id == data_id);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Always zero for some reason

johnramsden and others added 4 commits April 2, 2025 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants