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

Feat/af memory pruning #128

Merged
merged 18 commits into from
Jan 14, 2025
Merged

Feat/af memory pruning #128

merged 18 commits into from
Jan 14, 2025

Conversation

Xm0onh
Copy link
Member

@Xm0onh Xm0onh commented Jan 11, 2025

Following issue #127. Memory Pruner script with its test codes are in this PR. Now, memry pruner is a function at the end of the workflow.
The upper bound of the memory will be MAX_PROCESSED_IDS and MAX_PROCESSED_IDS over the last MAX_AGE_IN_DAYS.

P.S: This PR developed on top of two un-merged branches. We have to merge PRs #125, #126, and #130 first.

Copy link
Member

@jfrank-summit jfrank-summit left a comment

Choose a reason for hiding this comment

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

Thinking more about this, wouldn't it make more sense to handle any pruning in the state reducers? We can discuss on our call today.

@Xm0onh
Copy link
Member Author

Xm0onh commented Jan 13, 2025

I actually overlooked the current structure. The memory is not an issue for most of them because in reducer we only keep the new data. So we only need pruner for processedTweetIds.
Also, from memory management pov, we should prune at the stage we hit the limit, not defer it to end of workflow.

@Xm0onh Xm0onh requested a review from jfrank-summit January 13, 2025 22:24
@Xm0onh Xm0onh requested a review from jfrank-summit January 14, 2025 01:05
@Xm0onh Xm0onh mentioned this pull request Jan 14, 2025
@Xm0onh Xm0onh merged commit 45b5c0b into main Jan 14, 2025
1 check passed
@Xm0onh Xm0onh deleted the feat/af-memory-pruning branch January 14, 2025 16:34
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.

2 participants