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

Fixes index creation in wrong map-container problem. #7838

Merged

Conversation

Projects
None yet
4 participants
@ahmetmircik
Copy link
Member

commented Mar 25, 2016

Bug:
Upon subsequent destroy and creation of imap, there is a possibility that there can be more than one map-container referenced by different record-stores at the same time. Due to that, indexes can be created in an unexpected map-container and this can lead to return less than expected number of results when we query imap.

Fix:
Moved the removal of mapContainer to partition-thread, by making this, i want to be sure that after map#destroy, there is always a new mapContainer instance referenced from a newly created recordstore and that mapContainer instance is different from the removed one.

Test:
Testing this defect is so hard. I was able to reproduce it with adding some sleeps. But still trying to find a simple test also.

@ahmetmircik ahmetmircik added this to the 3.6.3 milestone Mar 25, 2016

@ahmetmircik

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2016

run-lab-run

1 similar comment
@ahmetmircik

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2016

run-lab-run

@ahmetmircik

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2016

Test Result (2 failures / -2)
com.hazelcast.client.heartbeat.ClientHeartbeatTest.testHeartbeatStoppedEvent
com.hazelcast.client.heartbeat.ClientHeartbeatTest.com.hazelcast.client.heartbeat.ClientHeartbeatTest
@ahmetmircik

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2016

run-lab-run

@sancar

This comment has been minimized.

Copy link
Member

commented Mar 25, 2016

@ahmetmircik These client test failures will be fixed by #7836

@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/3.6.3/mapContainerRefs branch 6 times, most recently from 1463dec to 21fe4ea Mar 29, 2016

@@ -284,6 +286,10 @@ public boolean isInvalidationEnabled() {
public InterceptorRegistry getInterceptorRegistry() {
return interceptorRegistry;
}

public boolean canBeRemoved() {

This comment has been minimized.

Copy link
@jerrinot

jerrinot Mar 30, 2016

Contributor

a very small thing: something like markForRemoval() could be better. as a developer I do not expect a method starting with can- to have a side-effect.

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Mar 30, 2016

Author Member

Thanks, completely removed that method, relying on mapContainers#remove(mapName, mapContainer) should be sufficient.

@ahmetmircik ahmetmircik changed the title [DONT MERGE] Fixes index creation in wrong map-container problem. Upon subsequent … [DONT MERGE] Fixes index creation in wrong map-container problem. Mar 30, 2016

Fixes index creation in wrong map-container problem. Upon subsequent …
…destroy and creation of imap, there is a possibility that there can be more than one map-container referenced by record-stores at the same time. Due to that indexes can be created in an unexpected map-container and this can lead to return less than expected number of results after querying imap

@ahmetmircik ahmetmircik force-pushed the ahmetmircik:fix/3.6.3/mapContainerRefs branch from 21fe4ea to 5579cb5 Mar 30, 2016

@ahmetmircik ahmetmircik changed the title [DONT MERGE] Fixes index creation in wrong map-container problem. Fixes index creation in wrong map-container problem. Mar 30, 2016

final PartitionContainer[] containers = partitionContainers;
final Semaphore semaphore = new Semaphore(0);
public void destroyMap(String mapName) {
MapContainer mapContainer = mapContainers.get(mapName);

This comment has been minimized.

Copy link
@jerrinot

jerrinot Mar 30, 2016

Contributor

which threads can call this method?
edit: I'll answer this myself. both partition threads & user thread can call this.

This comment has been minimized.

Copy link
@ahmetmircik

ahmetmircik Mar 30, 2016

Author Member

user + generic operation

@ahmetmircik

This comment has been minimized.

Copy link
Member Author

commented Mar 30, 2016

run-lab-run

@ahmetmircik

This comment has been minimized.

Copy link
Member Author

commented Mar 30, 2016

verify

@jerrinot

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2016

👍

1 similar comment
@eminn

This comment has been minimized.

Copy link
Collaborator

commented Mar 30, 2016

👍

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.