You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by dkatten <gi...@git.apache.org> on 2015/03/10 21:55:27 UTC
[GitHub] storm pull request: With hash key option for RedisMapState, only g...
GitHub user dkatten opened a pull request:
https://github.com/apache/storm/pull/462
With hash key option for RedisMapState, only get the values for keys in the requested batch
This commit fixes a bug whereby if the state updater is constructed with a hash key (ie, the state will be stored as a key in a redis hash, versus as a key in the top-level redis space), each call to multiGet would request the entire hash and iterate to extract only the values in the hash relevant to the batch.
This can cause an inordinate amount of network traffic (and actually caused our interfaces to fall over) for states with either a moderately high cardinality or large values. Instead, the call to Redis should be an hmget (hash multiget) that takes the hash key as its first argument and an array of strings as the keys to fetch from that key, thereby retrieving only the requested values.
The change also deprecates and removes buildValuesFromMap.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dkatten/storm redis-hgetall-fix
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/462.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #462
----
commit 26bab1595c56f6f40ea2392401e829f3ccb0cff0
Author: David Katten <dk...@geoforce.com>
Date: 2015-03-10T20:52:40Z
When using a hash key as an option for RedisMapState, only get the values for keys in the requested batch.
This commit fixes a bug whereby if the state updater is constructed with a hash key (ie, the state will be stored as a key in a redis hash, versus as a key in the top-level redis space), each call to multiGet would request the entire hash and iterate to extract only the values in the hash relevant to the batch.
This can cause an inordinate amount of network traffic (and actually caused our interfaces to fall over) for states with either a moderately high cardinality or large values. Instead, the call to Redis should be an hmget (hash multiget) that takes the hash key as its first argument and an array of strings as the keys to fetch from that key, thereby retrieving only the requested values.
The change also deprecates and removes buildValuesFromMap.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: With hash key option for RedisMapState, only g...
Posted by dkatten <gi...@git.apache.org>.
Github user dkatten commented on the pull request:
https://github.com/apache/storm/pull/462#issuecomment-78398611
STORM-703
https://issues.apache.org/jira/browse/STORM-703
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: With hash key option for RedisMapState, only g...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/462#issuecomment-78157556
@dkatten I've just missed it. Thanks! +1
@revans2 Could you take a look, please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-703] With hash key option for RedisMapS...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/462#issuecomment-85511919
@revans2 It's filed to JIRA so seems like we can merge it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-703] With hash key option for RedisMapS...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/462
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: With hash key option for RedisMapState, only g...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/462#issuecomment-78390669
The change looks good to me. I am +1 for the change. @dkatten if you want to file a JIRA under STORM at issues.apache.org that would be great. If you would rather I can file it for you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: With hash key option for RedisMapState, only g...
Posted by dkatten <gi...@git.apache.org>.
Github user dkatten commented on the pull request:
https://github.com/apache/storm/pull/462#issuecomment-78397688
I'll file it. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-703] With hash key option for RedisMapS...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/462#issuecomment-85513521
Sorry I really don't follow all the JIRa updates as closely as I should. @HeartSaVioR thanks for kicking me :) I'll merge it in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---