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

Set TTL attribute when soft-delete a sync item #22

Merged
merged 3 commits into from
Jun 9, 2020
Merged

Set TTL attribute when soft-delete a sync item #22

merged 3 commits into from
Jun 9, 2020

Conversation

yrliou
Copy link
Member

@yrliou yrliou commented Jun 9, 2020

Fix #21

@yrliou yrliou requested a review from husobee June 9, 2020 21:58
@yrliou yrliou self-assigned this Jun 9, 2020
Copy link

@husobee husobee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just the comment around the business effects around deleting items forever.

// items automatically by dynamoDB.
if *entity.Deleted {
update = update.Set(expression.Name(ttlAttrName), expression.Value(time.Now().Add(ttl).Unix()))
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means we will delete this forever, and will not be recoverable. I would suggest we create a cold storage s3 bucket where these soft deleted expired items exist. Or we need clear expectations to the users that soft delete means it will be deleted forever after 90 days.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point, will create an issue and kick-off a follow up discussion on this with @jsecretan and @hspencer77, we could definitely explore more options regarding the actions we want to take after this expired on dynamo, code would stay the same for go-sync here so merging.

@yrliou yrliou merged commit a07a84b into master Jun 9, 2020
@yrliou yrliou deleted the ttl branch June 9, 2020 22:47
yrliou added a commit that referenced this pull request Jun 10, 2020
This reverts commit a07a84b, reversing
changes made to 7f0c630.
yrliou added a commit that referenced this pull request Jun 10, 2020
Revert "Merge pull request #22 from brave/ttl"
@yrliou yrliou mentioned this pull request Jun 10, 2020
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.

Introduce cold storage for saving storage costs
2 participants