Skip to content

Use DynamoDB TTL to auto-expire old task results#5275

Open
crgwbr wants to merge 1 commit into
celery:mainfrom
crgwbr:feature_dynamodb_ttl
Open

Use DynamoDB TTL to auto-expire old task results#5275
crgwbr wants to merge 1 commit into
celery:mainfrom
crgwbr:feature_dynamodb_ttl

Conversation

@crgwbr

@crgwbr crgwbr commented Jan 8, 2019

Copy link
Copy Markdown

Description

Implement a TTL attribute on the DynamoDB result backend and configure the DB to
auto expire documents using the TTL.

@auvipy auvipy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you need to add tests for the proposed change.

Implement a TTL attribute on the DynamoDB result backend and configure the DB to
auto expire documents using the TTL.
@crgwbr
crgwbr force-pushed the feature_dynamodb_ttl branch from 73fd8a3 to 3b55881 Compare January 9, 2019 19:33
@crgwbr

crgwbr commented Jan 9, 2019

Copy link
Copy Markdown
Author

@auvipy @thedrow Added tests as requested.

@auvipy

auvipy commented Jan 10, 2019

Copy link
Copy Markdown
Member

check the flake8 error

@georgepsarakis georgepsarakis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work so far! I have noted some considerations, let me know if you need any help or clarifications.


def test_update_table_ttl_change_pending(self):
self.backend._client = MagicMock()
mock_describe_ttl = self.backend._client.describe_time_to_live = MagicMock()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

moto supports table TTL, so you could use it here instead of creating Mock objects, as a means of functional testing, especially since the local DynamoDB does not support TTL yet.

achieved_state = current_status == expected
sleep(1)

def _update_table_ttl(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this operation should be optional, since some users may want to retain results.

self._timestamp_field.data_type: str(_now)
},
self._ttl_field.name: {
self._ttl_field.data_type: str(int(_now) + int(self.expires))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps we should handle cases where expires is None/0 which disables expiration, as seen in the documentation?

What happens if you set NULL as the TTL field value, if this is possible? Would that perhaps disable expiration for this document?

Additionally, DynamoDB should be added in the documentation list of result backends supporting expiration.

@thedrow

thedrow commented Mar 3, 2019

Copy link
Copy Markdown
Contributor

@crgwbr Any chance you'll get to finish this before 4.3 GA?

@georgepsarakis

Copy link
Copy Markdown
Contributor

@crgwbr would you like to resume work here perhaps?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants