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

Use EvictionConfig for IMap eviction configuration #15592

Merged

Conversation

@ahmetmircik
Copy link
Member

ahmetmircik commented Sep 20, 2019

ee: hazelcast/hazelcast-enterprise#3317

Unifed IMap and ICache eviction configuration to decrease configuration complexity.

@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/4.0/sharedEvictionConfig branch 4 times, most recently from 30c897c to 5dca321 Sep 23, 2019
@ahmetmircik ahmetmircik changed the title wip WIP- Configure Map Eviction with EvictionConfig class Sep 24, 2019
@ahmetmircik ahmetmircik changed the title WIP- Configure Map Eviction with EvictionConfig class WIP-Configure Map Eviction with EvictionConfig class Sep 24, 2019
@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/4.0/sharedEvictionConfig branch 12 times, most recently from 0b3d3e8 to 852d83e Sep 25, 2019
@ahmetmircik ahmetmircik changed the title WIP-Configure Map Eviction with EvictionConfig class Configure Map Eviction with EvictionConfig class Oct 3, 2019
@ahmetmircik ahmetmircik marked this pull request as ready for review Oct 3, 2019
@ahmetmircik ahmetmircik requested a review from hazelcast/clients as a code owner Oct 3, 2019
@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/4.0/sharedEvictionConfig branch 3 times, most recently from 7775dd5 to 144f1f5 Oct 3, 2019
@mmedenjak mmedenjak added this to the 4.0 milestone Oct 8, 2019
@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/4.0/sharedEvictionConfig branch 2 times, most recently from 46d8c22 to 44eb21b Oct 8, 2019
@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/4.0/sharedEvictionConfig branch from 15c40bb to 413fcc7 Nov 2, 2019
@ahmetmircik

This comment has been minimized.

Copy link
Member Author

ahmetmircik commented Nov 4, 2019

@vbekiaris i also created ee counterpart: hazelcast/hazelcast-enterprise#3317

@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/4.0/sharedEvictionConfig branch 2 times, most recently from 91a7c77 to f97a495 Nov 4, 2019
@ahmetmircik ahmetmircik requested a review from vbekiaris Nov 4, 2019
@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/4.0/sharedEvictionConfig branch 2 times, most recently from fddee97 to b97334a Nov 4, 2019
private static void checkMapNativeMaxSizePolicy(MapConfig mapConfig) {
MaxSizePolicy maxSizePolicy = mapConfig.getEvictionConfig().getMaxSizePolicy();
if (!MAP_SUPPORTED_NATIVE_MAX_SIZE_POLICIES.contains(maxSizePolicy)) {
throw new IllegalArgumentException("Maximum size policy " + maxSizePolicy

This comment has been minimized.

Copy link
@mmedenjak

mmedenjak Nov 4, 2019

Contributor

Can we throw InvalidConfigurationException in this class instead?

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Nov 4, 2019

Author Member

done.

if (size != null) {
evictionConfig.setSize(parseInt(getTextContent(size)));
if (isIMap && evictionConfig.getSize() == 0) {

This comment has been minimized.

Copy link
@mmedenjak

mmedenjak Nov 4, 2019

Contributor

Why is this behaviour only on IMap?

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Nov 4, 2019

Author Member

I only know it is used only for IMap.

boolean isIMap,
boolean isNearCache) {
if (isIMap) {
checkMapMaxSizePolicyConfig(evictionConfig.getMaxSizePolicy());

This comment has been minimized.

Copy link
@mmedenjak

mmedenjak Nov 4, 2019

Contributor

Why don't we call checkEvictionConfig for IMap as well?

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Nov 4, 2019

Author Member

done.

checkEvictionConfig(evictionConfig);
checkCacheMaxSizePolicy(evictionConfig.getMaxSizePolicy(), inMemoryFormat);
// mergePolicyProvider is null when creating cache on client side.
if (mergePolicyProvider != null) {

This comment has been minimized.

Copy link
@mmedenjak

mmedenjak Nov 4, 2019

Contributor

Minor: You can omit this check as it's done in the checkMergeTypeProviderHasRequiredTypes as well.

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Nov 4, 2019

Author Member

fixed.

@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/4.0/sharedEvictionConfig branch 5 times, most recently from 7d1833c to fc11a48 Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.