Scrub persistence decode error #520

New Issue

Closed

opened 2023-03-06 15:17:24 +00:00 by jpds · 4 comments

jpds commented

2023-03-06 15:17:24 +00:00

Contributor

I appear to have missed a migration step in #516 - after deploying to my nodes, they hit:

ERROR garage_util::persister: Unable to decode persisted data file /meta/scrub_info

last-complete is epoch, but next-run is fine. But I can't see where in the code I can fix this or is it just a case of waiting for the next run and everything will be fine after that?

I deleted the file, and restarted the server - it doesn't get recreated, which is the bigger problem if the next-run is constantly pushed into the future.

I appear to have missed a migration step in #516 - after deploying to my nodes, they hit: ``` ERROR garage_util::persister: Unable to decode persisted data file /meta/scrub_info ``` `last-complete` is epoch, but `next-run` is fine. But I can't see where in the code I can fix this or is it just a case of waiting for the next run and everything will be fine after that? I deleted the file, and restarted the server - it doesn't get recreated, which is the bigger problem if the `next-run` is constantly pushed into the future.

lx commented

2023-03-06 17:21:43 +00:00

Owner

I think we don't want to lose info, especially the corruption counter, so we need to make a migration. Here is an example of a datastructure that supports a migration between versions:

https://git.deuxfleurs.fr/Deuxfleurs/garage/src/branch/main/src/model/key_table.rs

Basically, this is what happens :

the old version is kept in a mod named as the current version name or the version in which it was introduced (it's in a mod but not in a separate file, everything is in the same file, see example)
the declaration of the new version we are making is put in a mod named as the upcoming version
the old declaration implements garage_util::migrate::InitialFormat
the new declaration implements garage_util::migrate::Migrate, where it declares Previous (the struct of the previous version) and migrate (the function that transforms the old struct into the new struct)
there is a pub use vxxx::* that makes the new declaration directly usable in all the code (where xxx can be changed to match the latest version when more migrations are made)

Would you like to try doing this for the ScrubWorkerPersisted data structure, or should I do it myself?

I deleted the file, and restarted the server - it doesn't get recreated, which is the bigger problem if the next-run is constantly pushed into the future.

For this one, I think you can just call persister.save() once when the scrub worker is created

I think we don't want to lose info, especially the corruption counter, so we need to make a migration. Here is an example of a datastructure that supports a migration between versions: https://git.deuxfleurs.fr/Deuxfleurs/garage/src/branch/main/src/model/key_table.rs Basically, this is what happens : - the old version is kept in a mod named as the current version name or the version in which it was introduced (it's in a mod but not in a separate file, everything is in the same file, see example) - the declaration of the new version we are making is put in a mod named as the upcoming version - the old declaration implements `garage_util::migrate::InitialFormat` - the new declaration implements `garage_util::migrate::Migrate`, where it declares `Previous` (the struct of the previous version) and `migrate` (the function that transforms the old struct into the new struct) - there is a `pub use vxxx::*` that makes the new declaration directly usable in all the code (where xxx can be changed to match the latest version when more migrations are made) Would you like to try doing this for the `ScrubWorkerPersisted` data structure, or should I do it myself? > I deleted the file, and restarted the server - it doesn't get recreated, which is the bigger problem if the next-run is constantly pushed into the future. For this one, I think you can just call `persister.save()` once when the scrub worker is created

jpds referenced a pull request that will close this issue

2023-03-09 15:41:19 +00:00

block/repair.rs: Added migration for ScrubWorkerPersisted's time_next_run_scrub. #523

jpds commented

2023-03-09 16:55:49 +00:00

Author

Contributor

For this one, I think you can just call persister.save() once when the scrub worker is created

I cannot find a way to easy call this as this is a method of Persister and every part of the code seems to instead use PersisterShared. What should I do?

> For this one, I think you can just call persister.save() once when the scrub worker is created I cannot find a way to easy call this as this is a method of `Persister` and every part of the code seems to instead use `PersisterShared`. What should I do?

lx commented

2023-03-09 16:59:32 +00:00

Owner

As a hack I think you can do persister.set_with(|_| ());, that should do for now

As a hack I think you can do `persister.set_with(|_| ());`, that should do for now

lx commented

2023-03-09 17:00:33 +00:00

Owner

I'd add it here : https://git.deuxfleurs.fr/Deuxfleurs/garage/src/branch/main/src/block/manager.rs#L140