You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by anilkumar gingade <ag...@pivotal.io> on 2016/07/15 22:14:33 UTC
Review Request 50089: GEODE-1669: Performance issue with Interest
registration with impact on client side event processing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50089/
-----------------------------------------------------------
Review request for geode, anilkumar gingade, Barry Oglesby, Bruce Schuchardt, Jason Huynh, Lynn Hughes-Godfrey, Lynn Gallinat, William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
Repository: geode
Description
-------
GEODE-1669: Performance issue with Interest registration with impact on client side event processing
This is in relation to internal ticket GEM-437.
The client interests are managed in "FilterProfile" class on server side. These are maintained using the concurrent data structures CopyOnWriteHashSet and CopyOnWriteHashMap...
When set of keys are registered from client, the keys are added to CopyOnWriteHashSet one by one (FilterProfile.registerClientInterestList()); Where a new HashSet is created each time when an entry is added, which could impact performance based on the number of keys registered.
This will have implication on the client side event processing; as the event processing gets blocked when a interest registration is in progress by other thread.
Change/Solution:
Instead of adding the keys one by one; the keys are added using addAll(). This avoids creating one hashset per interest key, which was making the registration process to take longer time.
Diffs
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterProfile.java 08a6484
Diff: https://reviews.apache.org/r/50089/diff/
Testing
-------
Thanks,
anilkumar gingade
Re: Review Request 50089: GEODE-1669: Performance issue with Interest
registration with impact on client side event processing
Posted by anilkumar gingade <ag...@pivotal.io>.
> On July 15, 2016, 10:26 p.m., Bruce Schuchardt wrote:
> > I'm afraid that your changes alter the return value of these methods. They're only supposed to return keys that were newly registered. CacheClientProxy checks the return value to see if it should invoke registration-listeners. With your changes it will always think that all of the keys were newly registered and will always invoke listeners with the full set of keys. I think you need something more like this:
> >
> > keysRegistered = new HashSet(keys);
> > keysRegistered.removeAll(interestList.getSnapshot());
> > interestList.addAll(keys);
Myself and Barry talked about this...We thought, since its union of keys, if its already there it will be added to that...And invoking the listener will show all the events that gets added...
Your suggestion looks much better...I will make those changes...
- anilkumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50089/#review142456
-----------------------------------------------------------
On July 15, 2016, 10:14 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50089/
> -----------------------------------------------------------
>
> (Updated July 15, 2016, 10:14 p.m.)
>
>
> Review request for geode, anilkumar gingade, Barry Oglesby, Bruce Schuchardt, Jason Huynh, Lynn Hughes-Godfrey, Lynn Gallinat, William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1669: Performance issue with Interest registration with impact on client side event processing
>
> This is in relation to internal ticket GEM-437.
>
> The client interests are managed in "FilterProfile" class on server side. These are maintained using the concurrent data structures CopyOnWriteHashSet and CopyOnWriteHashMap...
>
> When set of keys are registered from client, the keys are added to CopyOnWriteHashSet one by one (FilterProfile.registerClientInterestList()); Where a new HashSet is created each time when an entry is added, which could impact performance based on the number of keys registered.
>
> This will have implication on the client side event processing; as the event processing gets blocked when a interest registration is in progress by other thread.
>
> Change/Solution:
>
> Instead of adding the keys one by one; the keys are added using addAll(). This avoids creating one hashset per interest key, which was making the registration process to take longer time.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterProfile.java 08a6484
>
> Diff: https://reviews.apache.org/r/50089/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 50089: GEODE-1669: Performance issue with Interest
registration with impact on client side event processing
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50089/#review142456
-----------------------------------------------------------
I'm afraid that your changes alter the return value of these methods. They're only supposed to return keys that were newly registered. CacheClientProxy checks the return value to see if it should invoke registration-listeners. With your changes it will always think that all of the keys were newly registered and will always invoke listeners with the full set of keys. I think you need something more like this:
keysRegistered = new HashSet(keys);
keysRegistered.removeAll(interestList.getSnapshot());
interestList.addAll(keys);
- Bruce Schuchardt
On July 15, 2016, 10:14 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50089/
> -----------------------------------------------------------
>
> (Updated July 15, 2016, 10:14 p.m.)
>
>
> Review request for geode, anilkumar gingade, Barry Oglesby, Bruce Schuchardt, Jason Huynh, Lynn Hughes-Godfrey, Lynn Gallinat, William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1669: Performance issue with Interest registration with impact on client side event processing
>
> This is in relation to internal ticket GEM-437.
>
> The client interests are managed in "FilterProfile" class on server side. These are maintained using the concurrent data structures CopyOnWriteHashSet and CopyOnWriteHashMap...
>
> When set of keys are registered from client, the keys are added to CopyOnWriteHashSet one by one (FilterProfile.registerClientInterestList()); Where a new HashSet is created each time when an entry is added, which could impact performance based on the number of keys registered.
>
> This will have implication on the client side event processing; as the event processing gets blocked when a interest registration is in progress by other thread.
>
> Change/Solution:
>
> Instead of adding the keys one by one; the keys are added using addAll(). This avoids creating one hashset per interest key, which was making the registration process to take longer time.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterProfile.java 08a6484
>
> Diff: https://reviews.apache.org/r/50089/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 50089: GEODE-1669: Performance issue with Interest
registration with impact on client side event processing
Posted by Jason Huynh <hu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50089/#review142460
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterProfile.java (line 565)
<https://reviews.apache.org/r/50089/#comment208024>
I think this goes along with Bruce's review, but possibily not as big of a problem, but the keysUnregistered may contain keys that were never actually in the interest list
- Jason Huynh
On July 15, 2016, 10:14 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50089/
> -----------------------------------------------------------
>
> (Updated July 15, 2016, 10:14 p.m.)
>
>
> Review request for geode, anilkumar gingade, Barry Oglesby, Bruce Schuchardt, Jason Huynh, Lynn Hughes-Godfrey, Lynn Gallinat, William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1669: Performance issue with Interest registration with impact on client side event processing
>
> This is in relation to internal ticket GEM-437.
>
> The client interests are managed in "FilterProfile" class on server side. These are maintained using the concurrent data structures CopyOnWriteHashSet and CopyOnWriteHashMap...
>
> When set of keys are registered from client, the keys are added to CopyOnWriteHashSet one by one (FilterProfile.registerClientInterestList()); Where a new HashSet is created each time when an entry is added, which could impact performance based on the number of keys registered.
>
> This will have implication on the client side event processing; as the event processing gets blocked when a interest registration is in progress by other thread.
>
> Change/Solution:
>
> Instead of adding the keys one by one; the keys are added using addAll(). This avoids creating one hashset per interest key, which was making the registration process to take longer time.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterProfile.java 08a6484
>
> Diff: https://reviews.apache.org/r/50089/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 50089: GEODE-1669: Performance issue with Interest
registration with impact on client side event processing
Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50089/#review142878
-----------------------------------------------------------
Ship it!
Ship It!
- Bruce Schuchardt
On July 19, 2016, 11:43 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50089/
> -----------------------------------------------------------
>
> (Updated July 19, 2016, 11:43 p.m.)
>
>
> Review request for geode, anilkumar gingade, Barry Oglesby, Bruce Schuchardt, Jason Huynh, Lynn Hughes-Godfrey, Lynn Gallinat, William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1669: Performance issue with Interest registration with impact on client side event processing
>
> This is in relation to internal ticket GEM-437.
>
> The client interests are managed in "FilterProfile" class on server side. These are maintained using the concurrent data structures CopyOnWriteHashSet and CopyOnWriteHashMap...
>
> When set of keys are registered from client, the keys are added to CopyOnWriteHashSet one by one (FilterProfile.registerClientInterestList()); Where a new HashSet is created each time when an entry is added, which could impact performance based on the number of keys registered.
>
> This will have implication on the client side event processing; as the event processing gets blocked when a interest registration is in progress by other thread.
>
> Change/Solution:
>
> Instead of adding the keys one by one; the keys are added using addAll(). This avoids creating one hashset per interest key, which was making the registration process to take longer time.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterProfile.java 08a6484
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/tier/sockets/FilterProfileJUnitTest.java 5d53dd2
>
> Diff: https://reviews.apache.org/r/50089/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 50089: GEODE-1669: Performance issue with Interest
registration with impact on client side event processing
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50089/
-----------------------------------------------------------
(Updated July 19, 2016, 11:43 p.m.)
Review request for geode, anilkumar gingade, Barry Oglesby, Bruce Schuchardt, Jason Huynh, Lynn Hughes-Godfrey, Lynn Gallinat, William Markito, nabarun nag, Dan Smith, and xiaojian zhou.
Changes
-------
Added changes based on review comments. Added new unit tests.
Repository: geode
Description
-------
GEODE-1669: Performance issue with Interest registration with impact on client side event processing
This is in relation to internal ticket GEM-437.
The client interests are managed in "FilterProfile" class on server side. These are maintained using the concurrent data structures CopyOnWriteHashSet and CopyOnWriteHashMap...
When set of keys are registered from client, the keys are added to CopyOnWriteHashSet one by one (FilterProfile.registerClientInterestList()); Where a new HashSet is created each time when an entry is added, which could impact performance based on the number of keys registered.
This will have implication on the client side event processing; as the event processing gets blocked when a interest registration is in progress by other thread.
Change/Solution:
Instead of adding the keys one by one; the keys are added using addAll(). This avoids creating one hashset per interest key, which was making the registration process to take longer time.
Diffs (updated)
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/FilterProfile.java 08a6484
geode-core/src/test/java/com/gemstone/gemfire/internal/cache/tier/sockets/FilterProfileJUnitTest.java 5d53dd2
Diff: https://reviews.apache.org/r/50089/diff/
Testing
-------
Thanks,
anilkumar gingade