Skip to content

Add argument to dont expire when loading from snapshot #3858

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

Closed
cfradewavecom opened this issue Oct 3, 2024 · 10 comments · Fixed by #4667
Closed

Add argument to dont expire when loading from snapshot #3858

cfradewavecom opened this issue Oct 3, 2024 · 10 comments · Fixed by #4667
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@cfradewavecom
Copy link

When restoring from an older snapshot redis keys with expire TTL are removed at start.
I understand that this maybe the default flow but in my opinion breaks the concept of snapshot.
My request is to add a new argument, for example --no-expire to allow all data from snapshot to be recovered.

@kostasrim
Copy link
Contributor

kostasrim commented Oct 3, 2024

Hi @cfradewavecom

I understand that this maybe the default flow but in my opinion breaks the concept of snapshot.

Why does it ? If an element has expired then why would I want to restore it ?

Does something similar exist for Valkey or ?

@cfradewavecom
Copy link
Author

Restoring everything (applications and cache) to the same date as snapshot may be expecting that a key with expiration is still there but when starting it is already gone from cache because it expired due to the date jump.

This may well be a design flaw from applications using cache but with that argument we could control the behavior and even recover important data from cache that would expire eventually when recovering from a snapshot. Right now even if i needed to look what data was in a snapshot i can't.

@kostasrim
Copy link
Contributor

kostasrim commented Oct 3, 2024

@cfradewavecom I genuinely don't see or understand why this is a problem ? You got a snapshot let's say at time X. Some of your data should expire at time X + 1 and then you loaded the snapshot at time X + 2. Why would the data you loaded include the expired elements since by design and definition of expires they should not exist after a certain time point?

This looks more of a design defect of an application. Simply, if you don't want certain elements to expire, then don't attach an expiration date to them.

@romange Any ideas why this would be useful ?

@cfradewavecom
Copy link
Author

cfradewavecom commented Oct 3, 2024

I agree with you but i cannot control the way applications are designed and this already happened to me where i could not recover from a disaster event because there where important information in a snapshot and i could not retrieve it has it is.

If i am not mistaken, this is supported with redis insight
Captura de ecrã de 2024-10-03 13-02-33

And a quick search there are several tools that can at least dump information from snapshot. If this feature does not make sense would be possible to provide a tool to dump information from snapshot with your current spec?

@kostasrim
Copy link
Contributor

Hi @cfradewavecom,

this already happened to me where i could not recover from a disaster event

That's why there is replication right ?

I am not familiar much with Redis Insight, but I guess if it can load an rdb fille you can do the same with DF, but saving into the old rdb format (and not the one we provide). However this will have a few downsides I am afraid

@cfradewavecom
Copy link
Author

it can load an rdb fille you can do the same with DF, but saving into the old rdb format (and not the one we provide). However this will have a few downsides I am afraid

Yes i notice that after the event and arrive to the same conclusion as you. The snapshot that i needed was in your native format.

That's why there is replication right ?
I had two replicas of dragonfly, both did their snapshot, at some point their volume run out of space since they do not remove older snapshots (i will create something to do this). Since this is a silent error (log only), i did not notice this was occurring. Suffered from restart on both replicas, they started from the last snapshot that was too old for the TTL that was defined in the keys at the time.

@adiholden adiholden added enhancement New feature or request help wanted Extra attention is needed labels Oct 6, 2024
@adiholden
Copy link
Collaborator

Thanks @cfradewavecom I think this is a good suggestion.
I added the 'help wanted' label, maybe an external contributor can take this.

The task is:

  1. introduce a new flag in rdb_load.cc
  2. ignore the expire info loaded from rdb file on load if flag is set to true

@adiholden adiholden added the good first issue Good for newcomers label Oct 6, 2024
@curioustien
Copy link

I can take a stab at this if no one is working on this task

@curioustien
Copy link

take

@romange
Copy link
Collaborator

romange commented Feb 17, 2025

Sure, why not.

H4R5H1T-007 added a commit to H4R5H1T-007/dragonfly that referenced this issue Feb 27, 2025
H4R5H1T-007 added a commit to H4R5H1T-007/dragonfly that referenced this issue Mar 2, 2025
Added a new flag --ignore-expiry to ignore key expiry when loading from RDB Snapshot. Also cached this flag into RDBLoader object to reuse it.
H4R5H1T-007 added a commit to H4R5H1T-007/dragonfly that referenced this issue Mar 4, 2025
Added a new flag --rdb_ignore_expiry to ignore key expiry when loading from RDB Snapshot. Also cached this flag into RDBLoader object to reuse it.
romange pushed a commit that referenced this issue Mar 4, 2025
feat(rdb_load): Added a flag to ignore key expiry #3858.

Added a new flag --rdb_ignore_expiry to ignore key expiry when loading from RDB Snapshot. Also cached this flag into RDBLoader object to reuse it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants