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

Added getAll() method to Near Cache serialization count tests #10723

Merged

Conversation

@Donnerbart
Copy link
Contributor

@Donnerbart Donnerbart commented Jun 8, 2017

We had no check for the getAll() method.

This also adds the shortcut to leave the getAllInternal() methods if all entries were fetched from the Near Cache (same as in the get() call). Without this we even do a remote call if all entries are in the Near Cache. This is super inefficient.

I also had to clone the collection of user keys, since we use iterator.remove() on them, to just fetch remote keys which are not in the Near Cache. If a user submits an modifiable collection the iterator.remove() fails with an UnsupportedOperationException.

@Donnerbart Donnerbart added this to the 3.9 milestone Jun 8, 2017
@Donnerbart Donnerbart self-assigned this Jun 8, 2017
@Donnerbart Donnerbart force-pushed the Donnerbart:addedGetAllSerializationCountTests branch 2 times, most recently from e286ad8 to bd91e84 Jun 8, 2017
@hazelcast hazelcast deleted a comment from devOpsHazelcast Jun 8, 2017
@hazelcast hazelcast deleted a comment from devOpsHazelcast Jun 8, 2017
@Donnerbart Donnerbart force-pushed the Donnerbart:addedGetAllSerializationCountTests branch 2 times, most recently from a6db2d9 to 9e8d861 Jun 8, 2017
@hazelcast hazelcast deleted a comment from devOpsHazelcast Jun 8, 2017
@hazelcast hazelcast deleted a comment from devOpsHazelcast Jun 8, 2017
@Donnerbart Donnerbart force-pushed the Donnerbart:addedGetAllSerializationCountTests branch from 709b214 to b8afab3 Jun 8, 2017
@hazelcast hazelcast deleted a comment from devOpsHazelcast Jun 8, 2017
@hazelcast hazelcast deleted a comment from devOpsHazelcast Jun 8, 2017
protected List<MapGetAllCodec.ResponseParameters> getAllInternal(Set<K> keys, Map<Integer, List<Data>> partitionToKeyData,
Map<K, V> result) {
Map<Object, Long> reservations = new HashMap<Object, Long>();
try {
if (serializeKeys) {
fillPartitionToKeyData(keys, partitionToKeyData);
boolean allEmpty = true;

This comment has been minimized.

@Donnerbart

Donnerbart Jun 8, 2017
Author Contributor

This is just a temporary fix to prevent the remote calls if all values are served from the Near Cache. A proper fix is made in #10714

@Donnerbart Donnerbart force-pushed the Donnerbart:addedGetAllSerializationCountTests branch from b8afab3 to 8d8af6d Jun 8, 2017
@devOpsHazelcast
Copy link
Contributor

@devOpsHazelcast devOpsHazelcast commented Jun 8, 2017

Test PASSed.

@hazelcast hazelcast deleted a comment from devOpsHazelcast Jun 8, 2017
Copy link
Contributor

@vbekiaris vbekiaris left a comment

Good catch!

@Donnerbart Donnerbart merged commit 6333701 into hazelcast:master Jun 9, 2017
1 check passed
1 check passed
default Build finished.
Details
@Donnerbart Donnerbart deleted the Donnerbart:addedGetAllSerializationCountTests branch Jun 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants