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

[3.7.3] Do not change keyLoaderState when loading keys supplied by user #9263

Conversation

Projects
None yet
4 participants
@jerrinot
Copy link
Contributor

commented Nov 16, 2016

The LoadAllOperation is used in 2 contexts:

  1. loadAll() - when all entries are reloaded
  2. loadAll(keys) - when reload of specific keys is requested by user

The first case uses a complex key-loading coordination and state transitions,
while the 2nd case is fairly simple - every member receives a set of keys
directly from the user.

This changeset fix the behaviour in the 2nd case where the
state was changed by accident and this led to a buggy
behaviour.

Fixes #9255
EE-part: hazelcast/hazelcast-enterprise#1135

Port of #9262 into the 3.7.3 release branch
(cherry picked from commit 734509d)

@jerrinot jerrinot added this to the 3.7.3 milestone Nov 16, 2016

return new LoadAllOperation(name, keys, replaceExistingValues);
public MapOperation createLoadAllOperation(String name, List<Data> keys, boolean replaceExistingValues,
boolean localOnlyLoad) {
return new LoadAllOperation(name, keys, replaceExistingValues, localOnlyLoad);

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Nov 16, 2016

Member

maybe we can use withUserSuppliedKeys instead of localOnlyLoad. localOnlyLoad seemed not clear

This comment has been minimized.

Copy link
@jerrinot

jerrinot Nov 16, 2016

Author Contributor

can do. @tombujok: WDYT?

This comment has been minimized.

Copy link
@tombujok

tombujok Nov 16, 2016

Contributor

withUserSuppliedKeys is much more readable. Thx for fixing this.

this.localOnlyLoad = localOnlyLoad;
// the localOnlyLoad field piggy-backs on Operation flags for serialization
// as the field was introduced in a patch release
setFlag(localOnlyLoad, BITMASK_LOAD_ALL_LOCAL_ONLY);

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Nov 16, 2016

Member

nice hack

This comment has been minimized.

Copy link
@jerrinot

jerrinot Nov 16, 2016

Author Contributor

I blame Tom for this evil genius-kind of hack! :)

This comment has been minimized.

Copy link
@tombujok

tombujok Nov 16, 2016

Contributor

Thx Jaromir, let's say it's our joint work ;)

@ahmetmircik
Copy link
Member

left a comment

👍 + minor comments

Do not change keyLoaderState when loading keys supplied by user
The LoadAllOperation is used in 2 contexts:
1. loadAll() - when all entries are reloaded
2. loadAll(keys) - when reload of specific keys is requested by user

The first case uses a complex key-loading coordination and state transitions,
while the 2nd case is fairly simple - every member receives a set of keys
directly from the user.

This changeset fix the behaviour in the 2nd case where the
state was changed by accident and this led to a buggy
behaviour.

Fixes #9255

(cherry picked from commit 734509d)

@jerrinot jerrinot force-pushed the jerrinot:fixes/load-all-keys-field-piggy-backbacking/3.7.3 branch from 56678f7 to 4614c94 Nov 16, 2016

@devOpsHazelcast

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2016

Test PASSed.

1 similar comment
@devOpsHazelcast

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2016

Test PASSed.

@tombujok tombujok merged commit 8166eea into hazelcast:3.7.3 Nov 16, 2016

1 check passed

default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.