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.

Sign up for GitHub

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

Jump to bottom

pageserver: downloads of evicted layers with pre-v1 local paths panics #7783

Closed
jcsp opened this issue May 16, 2024 · 0 comments · Fixed by #7786
Closed

pageserver: downloads of evicted layers with pre-v1 local paths panics #7783

jcsp opened this issue May 16, 2024 · 0 comments · Fixed by #7786
Labels
c/storage/pageserver Component: storage: pageserver t/bug Issue Type: Bug

Comments

@jcsp
Copy link
Collaborator

jcsp commented May 16, 2024

This is a regression in not-yet-deployed code via #7660

We load a layer with an old-style name (no generation), evict it, then download it again. The download code uses a new-style path but the old-style path is still in the Layer object.
@jcsp jcsp added t/bug Issue Type: Bug c/storage/pageserver Component: storage: pageserver labels May 16, 2024
@jcsp jcsp changed the title pageserver: downloads of v0 layers with v0 local paths panic pageserver: downloads of evicted layers with pre-v1 local paths panics May 16, 2024
@jcsp jcsp mentioned this issue May 16, 2024
Merged
5 tasks
jcsp added a commit that referenced this issue May 17, 2024
@jcsp
pageserver: fixes for layer path changes (#7786)
a8e6d25 
## Problem

- When a layer with legacy local path format is evicted and then
re-downloaded, a panic happened because the path downloaded by remote
storage didn't match the path stored in Layer.
- While investigating, I also realized that secondary locations would
have a similar issue with evictions.

Closes: #7783 

## Summary of changes

- Make remote timeline client take local paths as an input: it should
not have its own ideas about local paths, instead it just uses the layer
path that the Layer has.
- Make secondary state store an explicit local path, populated on scan
of local disk at startup. This provides the same behavior as for Layer,
that our local_layer_path is a _default_, but the layer path can
actually be anything (e.g. an old style one).
- Add tests for both cases.
a-masterov pushed a commit that referenced this issue May 20, 2024
@jcsp @a-masterov
pageserver: fixes for layer path changes (#7786)
2da5953 
## Problem

- When a layer with legacy local path format is evicted and then
re-downloaded, a panic happened because the path downloaded by remote
storage didn't match the path stored in Layer.
- While investigating, I also realized that secondary locations would
have a similar issue with evictions.

Closes: #7783 

## Summary of changes

- Make remote timeline client take local paths as an input: it should
not have its own ideas about local paths, instead it just uses the layer
path that the Layer has.
- Make secondary state store an explicit local path, populated on scan
of local disk at startup. This provides the same behavior as for Layer,
that our local_layer_path is a _default_, but the layer path can
actually be anything (e.g. an old style one).
- Add tests for both cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c/storage/pageserver Component: storage: pageserver t/bug Issue Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

pageserver: fixes for layer path changes neondatabase/neon
1 participant
@jcsp