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

Get rid of EphemeralFile::ephemeral_files_id #5038

Closed
problame opened this issue Aug 18, 2023 · 4 comments
Closed

Get rid of EphemeralFile::ephemeral_files_id #5038

problame opened this issue Aug 18, 2023 · 4 comments
Labels
c/storage/pageserver Component: storage: pageserver

Comments

@problame
Copy link
Contributor

Do we need a separate ephemeral file ID and page cache file ID, or could they be the same?

Originally posted by @hlinnaka in #4994 (comment)

@problame problame added c/storage/pageserver Component: storage: pageserver m/good_first_issue Moment: when doing your first Neon contributions labels Aug 18, 2023
@Reminiscent
Copy link
Contributor

Reminiscent commented Aug 23, 2023

Hi, I'd like to try this issue.

@hlinnaka
Copy link
Contributor

@problame commented at #4994 (comment):

This PR maintains the separate IDs for the sole purpose of file names: e0b8785

I'd prefer if we don't intermingle these.

Hmm, looking at the code as it is in 'main' now, I don't see a separate "ephemeral file id" anymore. I guess that you renamed it NEXT_FILENAME counter. I think that helped to reduce the confusion already.

We could replace NEXT_FILENAME with the page cache file ID. Might be handy during debugging that you could see the file ID directly in the filename. But doesn't really matter much.

@Reminiscent
Copy link
Contributor

@hlinnaka Thanks for your explanation. I think you are right. I'll try other issues.

@koivunej koivunej removed the m/good_first_issue Moment: when doing your first Neon contributions label Sep 13, 2023
@koivunej
Copy link
Member

@problame could you update this to reflect current state and perhaps restore good_first_issue, or close?

@problame problame closed this as not planned Won't fix, can't repro, duplicate, stale Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/storage/pageserver Component: storage: pageserver
Projects
None yet
Development

No branches or pull requests

4 participants