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

Added Redis Reactor, fixes #7 #30

Closed
wants to merge 7 commits into from

Conversation

jcrugzz
Copy link
Member

@jcrugzz jcrugzz commented Feb 12, 2013

Let me know what you think the key should be, a test will also be forthcoming.

@3rd-Eden
Copy link
Member

Wondering if bitmap would be more appropriate for storing these details/metrics in to redis. See related articles:

@jcrugzz
Copy link
Member Author

jcrugzz commented Feb 12, 2013

Ill check those out, thanks.

@jcrugzz
Copy link
Member Author

jcrugzz commented Feb 12, 2013

Ok I am starting to understand now. The question I have is can the event data be universally metricized? It seems like it would depend on what the data is and what its being piped through to then calculate a specific metric. I'm thinking that it the reactor may need to take a few more arguments in order to define that. Thoughts?

@indexzero
Copy link
Member

@jcrugzz Thanks for the pull-request! Here are my thoughts.

  • In the current implementation I believe that keys will be overwritten because data.time is not guaranteed to be unique (e.g. two data events in the same millisecond is very likely). To make this unique I would suggest process.hrtime() or data.service+data.time
  • I really like the feature suggested by @3rd-Eden, but there are a few different things to consider:
    1. Data loss: By definition these storage mechanisms would lose data because they would only be tracking data.metric. So all information in data.tags, data.description, etc would be lost. I'm OK with that.
    2. Tracking active resources: The examples provided are all about tracking active users. This is a valid scenario, but not the only scenario to consider. Using SETBIT: SETBIT data.service data.metric 1 seems appropriate, where the metric would be the uid of the resource to track activity on.
    3. Keeping time series counts: This is basically what graphite is for, but it seems to be that by using HINCRBY is a space and complexity acceptable solution in redis. For example given a single [data.service, data.time data.metric] tuple when we want to track yearly, monthly, daily, hourly:
  HINCRBY data.service 2012          data.metric
  HINCRBY data.service 2012-02       data.metric
  HINCRBY data.service 2012-02-12    data.metric
  HINCRBY data.service 2012-02-12-21 data.metric

Note: In the above all times (2012, 2012-02, etc) would derived from data.time.

@jcrugzz
Copy link
Member Author

jcrugzz commented Feb 13, 2013

@indexzero Yea so there definitely seems to be multiple routes this could go. Since graphite seems to be meant for the time series data (and has built in graphing capabilities which seems sweet), would the best route be to use this reactor to track the active resources? This seems like it would generate the most value.

@indexzero
Copy link
Member

@jcrugzz I would say all scenarios are valid. We can add the other ones later.

@jcrugzz
Copy link
Member Author

jcrugzz commented Feb 13, 2013

@indexzero fair enough. Once this scenario is good to go I'll open up an issue for the others.

@jcrugzz
Copy link
Member Author

jcrugzz commented Mar 22, 2013

Converted my changes to a base class that accepts a function in this manner:

// Sync
godot.reactor()
.redis(options, function (client, data) {
  // Do something with redis
  return data;
});
// Async
godot.reactor()
.redis(options, function (client, data, callback) {
  // Do something with redis asyncly
  process.nextTick(function () {
    callback(null, data);
  });
});

I should be able to implement those 3 redis use cases in their own reactors inheriting from this.

@indexzero
Copy link
Member

Cherry-picked. Thanks!

@indexzero indexzero closed this Mar 23, 2013
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.

None yet

3 participants