[ 
https://issues.apache.org/jira/browse/GEODE-5113?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16444816#comment-16444816
 ] 

John Blum edited comment on GEODE-5113 at 4/19/18 9:36 PM:
-----------------------------------------------------------

Hi [~dschneider] -

It is unlikely that this change will affect most applications (IMO), unless 
users/developers are explicitly handling the previously thrown 
{{UnsupportedOperationException}} from the {{EvictionAttributes.getMaximum()}} 
method when called and LRU Heap Eviction is configured, which is technically 
possible, no matter how unlikely.  If so, clearly this will no will longer be 
"handled" in the way that the user expects.

I actually agree with this change, partially.

When I informed the GemFire PM team of this issue/change in API (behavior), 
which broke some tests in SDG's test suite, where SDG was configuring LRU Heap 
Eviction and expecting this behavior when calling 
{{EvictionAttributes.getMaximum()}}, it was to point out that this API change 
was not "properly documented", in Javadoc on the 
[{{EvictionAttributes.getMaximum()}}|http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/EvictionAttributes.html]
 method.

Unfortunately, the {{UnsupportedOperationException}} was never properly 
documented in Javadoc either, i.e. by declaring the Exception in a {{@throws}} 
tag on the method.  As a result, I had to dig into Geode's source to figure out 
what hand changed when the Exception was no longer thrown, which is when I 
realized the method [now returned 
0|https://github.com/apache/geode/blob/rel/v1.5.0/geode-core/src/main/java/org/apache/geode/internal/cache/EvictionAttributesImpl.java#L95-L101]
 vs. [throwing the 
{{UnsupportedOperationException}}|https://github.com/apache/geode/blob/rel/v1.4.0/geode-core/src/main/java/org/apache/geode/internal/cache/EvictionAttributesImpl.java#L138-L144]
 as before , <sigh>).

Now, why do I partially agree with this change?

Well,  I am wondering, would it make more sense to return 
[{{Cache.getResourceManager().getEvictionHeapPercentage()}}|http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/control/ResourceManager.html#getEvictionHeapPercentage--]
 instead since that is the setting affecting LRU Heap Eviction?

Food for thought,
-John





was (Author: jblum):
Hi [~dschneider] -

It is unlikely that this change will affect most applications (IMO), unless 
users/developers are explicitly handling the previously thrown 
{{UnsupportedOperationException}} from the {{EvictionAttributes.getMaximum()}} 
method when called and LRU Heap Eviction is configured, which is technically 
possible, no matter how unlikely.  If so, clearly this will no will longer be 
"handled" in the way that the user expects.

I actually agree with this change, partially.

When I informed the GemFire PM team of this issue/change in API (behavior), 
which broke some tests in SDG's test suite, where SDG was configuring LRU Heap 
Eviction and expecting this behavior when calling 
{{EvictionAttributes.getMaximum()}}, it was to point out that this API change 
was not "properly documented", in Javadoc on the 
[{{EvictionAttributes.getMaximum()}}|http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/EvictionAttributes.html]
 method.

Unfortunately, the {{UnsupportedOperationException}} was never properly 
documented in Javadoc either, i.e. by declaring the Exception in a {{@throws}} 
tag on the method.  As a result, I had to dig into Geode's source to figure out 
what hand changed when the Exception was no longer thrown, which is when I 
realized the method [now returned 
0|https://github.com/apache/geode/blob/rel/v1.5.0/geode-core/src/main/java/org/apache/geode/internal/cache/EvictionAttributesImpl.java#L95-L101]
 vs. 
[throwing|https://github.com/apache/geode/blob/rel/v1.4.0/geode-core/src/main/java/org/apache/geode/internal/cache/EvictionAttributesImpl.java#L138-L144]
 the {{UnsupportedOperationException}} previously, <sigh>).

Now, why do I partially agree with this change?

Well,  I am wondering, would it make more sense to return 
[{{Cache.getResourceManager().getEvictionHeapPercentage()}}|http://geode.apache.org/releases/latest/javadoc/org/apache/geode/cache/control/ResourceManager.html#getEvictionHeapPercentage--]
 instead since that is the setting affecting LRU Heap Eviction?

Food for thought,
-John




> EvictionAttributes.getMaximum() no longer throws 
> UnsupportedOperationException for LRU Heap
> -------------------------------------------------------------------------------------------
>
>                 Key: GEODE-5113
>                 URL: https://issues.apache.org/jira/browse/GEODE-5113
>             Project: Geode
>          Issue Type: Bug
>          Components: eviction
>            Reporter: Fred Krone
>            Priority: Major
>
>  
> Previously, the EvictionAttributes.getMaximum() used to throw an 
> UnsupportedOperationException if the user tried to configure a Maximum on an 
> LRU Heap Eviction Policy (Apache Geode 1.4).  Now Geode (and by extension, 
> GemFire) will just silently return 0.
>  
> in 1.4
> [https://github.com/apache/geode/blob/rel/v1.4.0/geode-core/src/main/java/org/apache/geode/internal/cache/EvictionAttributesImpl.java#L138-L144]
>  
> in 1.5
> [https://github.com/apache/geode/blob/rel/v1.5.0/geode-core/src/main/java/org/apache/geode/internal/cache/EvictionAttributesImpl.java#L95-L101]
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to