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

[BACKPORT] loadAll(keys) has to notify RecordStores about the end of loading #9260

Conversation

Projects
None yet
3 participants
@jerrinot
Copy link
Contributor

commented Nov 16, 2016

When a RecordStore process LoadAllOperation it expects to receive
LoadStatusOperation eventually. The LoadStatusOperation indicates
there are no more keys to be loaded. Without this the record store never
change its state into the LOADED state -> subsequent loadAll() calls are ignored

A candidate for #9255

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

loadAll(keys) has to notify RecordStores about the end of loading
When a RecordStore process LoadAllOperation it expects to receive
LoadStatusOperation eventually. The LoadStatusOperation indicates
there are no more keys to be loaded. Without this the record store never
change its state int the Loaded state -> subsequent loadAll() calls are ignored

Fixes #9255

@jerrinot jerrinot force-pushed the jerrinot:fixes/load-all-keys/maintanance-3.x branch from 0b8ecdf to ce4103e Nov 16, 2016

@devOpsHazelcast

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2016

Test PASSed.

@tombujok

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2016

The PR looks good, the only thing that is lacking to me is some documentation/description why we needed to add it, etc. I can imagine we will forget it in a couple of months.

@tombujok

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2016

One more comment is that there's interference between loadAll and loadAll(keys) with this fix.
The worst thing that could happen is:

  • you do (A.) loadAll which will load all keys (like 10M)
  • while the abovementioned call is in progress you do (B.) loadAll (with 1000 keys)
  • when B finishes it updates the loading status on all partitions so it means (A.) will think that the load has finished, even though (A.) may be still in progress for a long time.

@jerrinot jerrinot closed this Nov 16, 2016

@jerrinot

This comment has been minimized.

Copy link
Contributor Author

commented Nov 16, 2016

closed in favour of #9262

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.