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

feature: node cache/storage expiration #1727

Merged
merged 3 commits into from Nov 28, 2018
Merged

feature: node cache/storage expiration #1727

merged 3 commits into from Nov 28, 2018

Conversation

comountainclimber
Copy link
Member

@comountainclimber comountainclimber commented Nov 27, 2018

What current issue(s) from Trello/Github does this address?
This PR begins the iterative process of issue/enhancement #1712 - Specifically it adds an expiration to the "best node" algo and also sets an expiration on manually set nodes... This will resolve a number of possible latent issues around the current set up

What problem does this PR solve?
Because nodes could potentially fall behind go offline etc leaving them in memory (or local storage) permanently is generally a bad idea

How did you solve this problem?
node url is set in storage with a timestamp which is always compared to the current time... if its expired the node selection algorithm is run

How did you make sure your solution works?
By manual and automated test coverage

Are there any special changes in the code that we should be aware of?

Is there anything else we should know?

  • Unit tests written? 😎

@comountainclimber
Copy link
Member Author

Thanks for the solid review @evgenyboxer 💪

Copy link
Collaborator

@dvdschwrtz-zz dvdschwrtz-zz left a comment

Choose a reason for hiding this comment

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

You are doing great with your PRs. This looks really good

@comountainclimber
Copy link
Member Author

Thanks @dvdschwrtz 😄

@comountainclimber comountainclimber merged commit fbbdd70 into dev Nov 28, 2018
@comountainclimber comountainclimber deleted the feature/1712 branch December 15, 2018 19:46
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.

None yet

3 participants