Refactor #2
Open
Refactor #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Mother of all pull requests. I'll review it in the morning, thanks for taking the effort! |
|
|
||
| if (fn) fn(err, metroplex.address); | ||
| }); | ||
| metroplex.redis.psetex(metroplex.namespace + 'server:' + metroplex.address, metroplex.interval, Date.now(), function(err, result) { |
There was a problem hiding this comment.
I know that redis commands can fail only if called with a wrong syntax or against keys holding the wrong data type, but I would handle the error anyway, in the same way it was handled before.
|
@parrish after ~8 months I've finally taken a look at this pr, sorry about that. The biggest difference lies in how the sparks are stored in Redis. Right now they are stored in a hash to optimize the memory. With this patch instead, each spark has its own key, making it possible to add an individual timeout to each one of these. I let @3rd-Eden decide what to do with this pr. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
I'm not sure if metroplex was previously working as intended or not. When I tried it, all of the keys in Redis were deleted after the interval had expired except for the server:alive keys.
I started trying to fix it and ended up writing a large-ish refactor of how it uses Redis. I've also setup a spark TTL heartbeat using the client ping transmissions and expanded the tests a bit.
Feedback, questions, and suggestions are definitely welcome.