You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@geode.apache.org by "John Blum (JIRA)" <ji...@apache.org> on 2018/04/19 21:37:00 UTC

[jira] [Comment Edited] (GEODE-5113) EvictionAttributes.getMaximum() no longer throws UnsupportedOperationException for LRU Heap

    [ 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 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




> 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)