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

Fail-fast when custom merge policy is not appropriate #12284

Closed
ahmetmircik opened this issue Feb 8, 2018 · 7 comments
Closed

Fail-fast when custom merge policy is not appropriate #12284

ahmetmircik opened this issue Feb 8, 2018 · 7 comments

Comments

@ahmetmircik
Copy link
Member

@ahmetmircik ahmetmircik commented Feb 8, 2018

now failure happens when trying to merge clusters, seems too late to detect a wrongly implemented merge policy

For example this works without exception now:

Config config = new Config();
QueueConfig queueConfig = config.getQueueConfig("test");
queueConfig.getMergePolicyConfig().setPolicy(Object.class.getName());

HazelcastInstance instance = Hazelcast.newHazelcastInstance(config);
IQueue<String> queue = instance.getQueue("test");
queue.offer("one");
@Donnerbart
Copy link
Contributor

@Donnerbart Donnerbart commented Feb 8, 2018

This is already a checkpoint in the overview issue and will be fixed soon by PR: #11969

@mmedenjak
Copy link
Contributor

@mmedenjak mmedenjak commented Feb 19, 2018

@Donnerbart is this fixed by #12347?

@Donnerbart
Copy link
Contributor

@Donnerbart Donnerbart commented Feb 19, 2018

Oh, it could be that I forgot to add that "is no merge policy at all" check. Let me try this, otherwise I'll prepare a fix.

@Donnerbart
Copy link
Contributor

@Donnerbart Donnerbart commented Feb 19, 2018

Yeah, the check is missing, but it's not sooo easy to implement. The config is just a String and in the ConfigValidator we don't have the NodeEngine to access the SplitBrainMergePolicyProvider. I'll try to figure out a clean solution for this.

@ahmetmircik
Copy link
Member Author

@ahmetmircik ahmetmircik commented Mar 12, 2018

@Donnerbart maybe we can also check it in this method:

private static boolean canMergeWithPolicy(String mergePolicyClassName,

@Donnerbart
Copy link
Contributor

@Donnerbart Donnerbart commented Mar 12, 2018

After the next design improvement, there might anyway be other checks necessary (e.g. if the data structure can provide the required value types which the merge policy needs). So this is still on my todo, yes.

@Donnerbart Donnerbart self-assigned this Mar 19, 2018
@Donnerbart
Copy link
Contributor

@Donnerbart Donnerbart commented Mar 19, 2018

Will be fixed by #12573

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.