Skip to content

DATAREDIS-1955 - Phantom copy is not persisted when entity ttl is changed from positive to negative, causing phantom copy not updated to no expire and counting down, then RedisKeyExpiredEvent publishing #1961

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
Closed
wants to merge 3 commits into from
Closed

DATAREDIS-1955 - Phantom copy is not persisted when entity ttl is changed from positive to negative, causing phantom copy not updated to no expire and counting down, then RedisKeyExpiredEvent publishing #1961

wants to merge 3 commits into from

Conversation

muchnik
Copy link
Contributor

@muchnik muchnik commented Feb 8, 2021

DATAREDIS-1955 - Phantom copy is not persisted when entity ttl is changed from positive to negative, causing phantom copy not updated to no expire and counting down, then RedisKeyExpiredEvent publishing.

When updating an object that has a positive time to live to a negative time to live you must persist the phantom key.
Otherwise, after expiring time to live of the phantom, which was created when object with a positive time to live was saved - the indices are dropped and the keys are deleted from the sets as if entity is expired.

  • You have read the Spring Data contribution guidelines.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

@muchnik
DATAREDIS-1955 - Phantom copy is not persisted when entity ttl is cha…
2bd0f78 
…nged from positive to negative, causing phantom copy not updated to no expire and counting down, then RedisKeyExpiredEvent publishing

When updating an object that has a positive time to live to a negative time to live you must persist the phantom key.
Otherwise, after expiring time to live of the phantom, which was created when object with a positive time to live was saved - the indices are dropped and the keys are deleted from the sets as if entity is expired.
@muchnik
Added myself as author to touched classes.
d6de017
@muchnik muchnik mentioned this pull request Feb 8, 2021
Closed
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 8, 2021
@christophstrobl christophstrobl self-assigned this Feb 10, 2021
@christophstrobl christophstrobl added in: repository Repositories abstraction type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 10, 2021
@christophstrobl christophstrobl linked an issue Feb 10, 2021 that may be closed by this pull request
RedisKeyExpiredEvent is published when entity expiring was changed from positive to -1, resulting in dropping meta information (indexes etc) #1955
Closed
@christophstrobl
Copy link
Member

@muchnik thanks for the PR!
We can take it from here - I tend to switch from persisting the phantom key to removing it (also for the update scenario). This would reduce the footprint on server side. Objections?

@muchnik
In order to reduce the footprint on server side in case of update and…
b7853f2 
… partial update, when phantom key is not needed - it now deletes instead of persisting.
@muchnik
Copy link
Contributor Author

muchnik commented Feb 10, 2021

@muchnik thanks for the PR!
We can take it from here - I tend to switch from persisting the phantom key to removing it (also for the update scenario). This would reduce the footprint on server side. Objections?

Nice idea, updated PR!

christophstrobl pushed a commit that referenced this pull request Feb 16, 2021
@muchnik @christophstrobl
Remove phantom copy of expiring data when source gets persisted.
8933e02 
This commit makes sure to clean up resources when a previously expiring entity is persisted by setting the time to live to zero or negative.
In case a phantom copy exists for the changed entity, it is removed to free space on the server and prevent expiration events from being sent.

See #1955
Original Pull Request: #1961

# Conflicts:
#	src/test/java/org/springframework/data/redis/core/RedisKeyValueAdapterTests.java
christophstrobl added a commit that referenced this pull request Feb 16, 2021
@christophstrobl
Move expires check to dedicated method.
9b18e0c 
Introduce helper method to unify expiry check and simplify control flow.

Original Pull Request: #1961
christophstrobl pushed a commit that referenced this pull request Feb 16, 2021
@muchnik @christophstrobl
Remove phantom copy of expiring data when source gets persisted.
3f4df6d 
This commit makes sure to clean up resources when a previously expiring entity is persisted by setting the time to live to zero or negative.
In case a phantom copy exists for the changed entity, it is removed to free space on the server and prevent expiration events from being sent.

See #1955
Original Pull Request: #1961
christophstrobl added a commit that referenced this pull request Feb 16, 2021
@christophstrobl
Move expires check to dedicated method.
5d3d212 
Introduce helper method to unify expiry check and simplify control flow.

Original Pull Request: #1961
christophstrobl pushed a commit that referenced this pull request Feb 16, 2021
@muchnik @christophstrobl
Remove phantom copy of expiring data when source gets persisted.
e462467 
This commit makes sure to clean up resources when a previously expiring entity is persisted by setting the time to live to zero or negative.
In case a phantom copy exists for the changed entity, it is removed to free space on the server and prevent expiration events from being sent.

Closes #1955
Original Pull Request: #1961
christophstrobl added a commit that referenced this pull request Feb 16, 2021
@christophstrobl
Move expires check to dedicated method.
4df8e44 
Introduce helper method to unify expiry check and simplify control flow.

Original Pull Request: #1961
@christophstrobl
Copy link
Member

Thanks @muchnik! Merged to main branch and back ported to 2.3.x and 2.4.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@christophstrobl christophstrobl

Labels
in: repository Repositories abstraction type: bug A general bug
Projects
None yet
Milestone
No milestone
3 participants
@muchnik @christophstrobl @spring-projects-issues