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

fix issue #1597: mark files being written as "active" #1671

Closed
wants to merge 2 commits into from

Conversation

facontidavide
Copy link

@facontidavide facontidavide commented May 22, 2024

Suggested changes:

  1. Add the suffix ".active" to SequentialWriter::format_storage_uri
  2. When storage_ is closed, rename the file to remove the ".active" suffix.
  3. Save the metadata when splitting. In this way all the parts but the last one are still usable in case of crash.

To address #1597 point 3 is not necessary and maybe not even desirable, so let me know what you think.

Note:

Since rcpputils::fs is not wrapping "rename", I had to use std::filesystem instead and upgrade to C++ 17.

@facontidavide facontidavide requested a review from a team as a code owner May 22, 2024 14:46
@facontidavide facontidavide requested review from MichaelOrlov and jhdcs and removed request for a team May 22, 2024 14:46
@fujitatomoya
Copy link
Contributor

@facontidavide thanks for the PR.

@MichaelOrlov are we supposed to create PR target to rolling 1st? and then backport?

@clalancette
Copy link
Contributor

@MichaelOrlov are we supposed to create PR target to rolling 1st? and then backport?

Yes please. Also, we are going to run into other problems with the switch to std::filesystem on Humble (because of older GCC compilers). But let's target this to rolling first, then deal with that in the backport.

@facontidavide
Copy link
Author

I am temporarily closing this in favor of #1672

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