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

IMap containsKey() Does Not Reset Idle Timeout #472

Closed
PixelRaith opened this issue May 1, 2013 · 5 comments

Comments

Projects
None yet
3 participants
@PixelRaith
Copy link
Contributor

commented May 1, 2013

This is similar to issue #288, only for IMap instead of CMap. It seems that calling containsKey() on an IMap does not reset the idle timeout for that entry. My usage scenario is pretty much identical to the one in the related issue.

Am I correct in thinking that this can be fixed by patching ConcurrentMapManager as follows?

public boolean containsKey(String name, Object key) {
    this.keyObject = key;
    this.nearCache = mapCaches.get(name);
    Data dataKey = toData(key);
    if (nearCache != null) {
        if (nearCache.containsKey(key)) {
            return true;
        }
    }
    final CMap cMap = maps.get(name);
    if (cMap != null) {
        Record record = cMap.getOwnedRecord(dataKey);
        if (record != null && record.isActive() && record.isValid() && record.hasValueData()) {
            if (cMap.isReadBackupData()) {
+               record.setLastAccessed();
                return true;
            } else {
                PartitionServiceImpl.PartitionProxy partition = PartitionServiceImpl.getPartition(record.getBlockId());
                if (partition != null && !partitionManager.isOwnedPartitionMigrating(partition.getPartitionId())
                        && partition.getOwner() != null && partition.getOwner().localMember()) {
+                   record.setLastAccessed();
                    return true;
                }
            }
        }
    }
    return booleanCall(CONCURRENT_MAP_CONTAINS_KEY, name, dataKey, null, 0, -1);
}

Note that I'm not sure how this should be dealt with if the entry in question is in the NearCache, and due to the nature of NearCache if it's even an issue. Thus, this may well only be a partial fix.

I have written a couple of unit tests to check that IMap's get() and containsKey() functions both reset the idle timeout (package/imports omitted):

@RunWith(com.hazelcast.util.RandomBlockJUnit4ClassRunner.class)
public class IMapTest extends TestUtil {

    @BeforeClass
    public static void init() throws Exception {
        System.setProperty(GroupProperties.PROP_WAIT_SECONDS_BEFORE_JOIN, "1");
        System.setProperty(GroupProperties.PROP_VERSION_CHECK_ENABLED, "false");
        Hazelcast.shutdownAll();
    }

    @Before
    @After
    public void cleanup() throws Exception {
        Hazelcast.shutdownAll();
    }


    @Test
    public void testContainsKeyUpdatesLastAccessTime() throws Exception {
        Config config = new Config();
        FactoryImpl mockFactory = mock(FactoryImpl.class);
        // we mocked the node
        // do not forget to shutdown the connectionManager
        // so that server socket can be released.
        Node node = new Node(mockFactory, config);
        node.serviceThread = Thread.currentThread();
        IMap imap = Hazelcast.getMap("c:myMap");
        Object key = "1";
        Object value = "istanbul";
        Data dKey = toData(key);
        Data dValue = toData(value);
        imap.put(dKey, dValue);
        MapEntry entry = imap.getMapEntry(dKey);
        long firstAccessTime = entry.getLastAccessTime();
        Thread.sleep(10);
        assertTrue(imap.containsKey(dKey));
        entry = imap.getMapEntry(dKey);
        long secondAccessTime = entry.getLastAccessTime();
        assertTrue("entry access time should have been updated on containsKey()", secondAccessTime > firstAccessTime);
        node.connectionManager.shutdown();
    }

    @Test
    public void testGetUpdatesLastAccessTime() throws Exception {
        Config config = new Config();
        FactoryImpl mockFactory = mock(FactoryImpl.class);
        // we mocked the node
        // do not forget to shutdown the connectionManager
        // so that server socket can be released.
        Node node = new Node(mockFactory, config);
        node.serviceThread = Thread.currentThread();
        IMap imap = Hazelcast.getMap("c:myMap");
        Object key = "1";
        Object value = "istanbul";
        Data dKey = toData(key);
        Data dValue = toData(value);
        imap.put(dKey, dValue);
        MapEntry entry = imap.getMapEntry(dKey);
        long firstAccessTime = entry.getLastAccessTime();
        Thread.sleep(10);
        assertTrue(imap.get(dKey).equals(value));
        entry = imap.getMapEntry(dKey);
        long secondAccessTime = entry.getLastAccessTime();
        assertTrue("entry access time should have been updated on get()", secondAccessTime > firstAccessTime);
        node.connectionManager.shutdown();
    }
}

Locally, I put them in a new class IMapTest because it didn't feel right to put them in IMapAsyncTest. The tests are heavily based upon both the unit tests in place, and the unit tests accompanying the fix for issue #288.

If this is indeed an issue, and if my fix is correct, I'd be more than happy to provide it in a pull request if this is easier.

Many thanks,
Raith

@enesakar

This comment has been minimized.

Copy link
Member

commented May 3, 2013

Thanks, your fix seems to resolve the issue. But I am not sure if
containsKey operation should prevent the idle eviction. It seems to me as
just query on map and does not touch the entry. But I am not sure so will
talk this with the team.

On Wed, May 1, 2013 at 5:12 PM, TheDastardlyRaith
notifications@github.comwrote:

This is similar to issue #288#288,
only for IMap instead of CMap. It seems that calling containsKey() on an
IMap does not reset the idle timeout for that entry. My usage scenario is
pretty much identical to the one in the related issue.

Am I correct in thinking that this can be fixed by patching
ConcurrentMapManager as follows?

public boolean containsKey(String name, Object key) {
this.keyObject = key;
this.nearCache = mapCaches.get(name);
Data dataKey = toData(key);
if (nearCache != null) {
if (nearCache.containsKey(key)) {
return true;
}
}
final CMap cMap = maps.get(name);
if (cMap != null) {
Record record = cMap.getOwnedRecord(dataKey);
if (record != null && record.isActive() && record.isValid() && record.hasValueData()) {
if (cMap.isReadBackupData()) {+ record.setLastAccessed();
return true;
} else {
PartitionServiceImpl.PartitionProxy partition = PartitionServiceImpl.getPartition(record.getBlockId());
if (partition != null && !partitionManager.isOwnedPartitionMigrating(partition.getPartitionId())
&& partition.getOwner() != null && partition.getOwner().localMember()) {+ record.setLastAccessed();
return true;
}
}
}
}
return booleanCall(CONCURRENT_MAP_CONTAINS_KEY, name, dataKey, null, 0, -1);}

Note that I'm not sure how this should be dealt with if the entry in
question is in the NearCache, and due to the nature of NearCache if it's
even an issue. Thus, this may well only be a partial fix.

I have written a couple of unit tests to check that IMap's get() and
containsKey() functions both reset the idle timeout (package/imports
omitted):

@RunWith(com.hazelcast.util.RandomBlockJUnit4ClassRunner.class)public class IMapTest extends TestUtil {

@BeforeClass
public static void init() throws Exception {
    System.setProperty(GroupProperties.PROP_WAIT_SECONDS_BEFORE_JOIN, "1");
    System.setProperty(GroupProperties.PROP_VERSION_CHECK_ENABLED, "false");
    Hazelcast.shutdownAll();
}

@Before
@After
public void cleanup() throws Exception {
    Hazelcast.shutdownAll();
}


@Test
public void testContainsKeyUpdatesLastAccessTime() throws Exception {
    Config config = new Config();
    FactoryImpl mockFactory = mock(FactoryImpl.class);
    // we mocked the node
    // do not forget to shutdown the connectionManager
    // so that server socket can be released.
    Node node = new Node(mockFactory, config);
    node.serviceThread = Thread.currentThread();
    IMap imap = Hazelcast.getMap("c:myMap");
    Object key = "1";
    Object value = "istanbul";
    Data dKey = toData(key);
    Data dValue = toData(value);
    imap.put(dKey, dValue);
    MapEntry entry = imap.getMapEntry(dKey);
    long firstAccessTime = entry.getLastAccessTime();
    Thread.sleep(10);
    assertTrue(imap.containsKey(dKey));
    entry = imap.getMapEntry(dKey);
    long secondAccessTime = entry.getLastAccessTime();
    assertTrue("entry access time should have been updated on containsKey()", secondAccessTime > firstAccessTime);
    node.connectionManager.shutdown();
}

@Test
public void testGetUpdatesLastAccessTime() throws Exception {
    Config config = new Config();
    FactoryImpl mockFactory = mock(FactoryImpl.class);
    // we mocked the node
    // do not forget to shutdown the connectionManager
    // so that server socket can be released.
    Node node = new Node(mockFactory, config);
    node.serviceThread = Thread.currentThread();
    IMap imap = Hazelcast.getMap("c:myMap");
    Object key = "1";
    Object value = "istanbul";
    Data dKey = toData(key);
    Data dValue = toData(value);
    imap.put(dKey, dValue);
    MapEntry entry = imap.getMapEntry(dKey);
    long firstAccessTime = entry.getLastAccessTime();
    Thread.sleep(10);
    assertTrue(imap.get(dKey).equals(value));
    entry = imap.getMapEntry(dKey);
    long secondAccessTime = entry.getLastAccessTime();
    assertTrue("entry access time should have been updated on get()", secondAccessTime > firstAccessTime);
    node.connectionManager.shutdown();
}}

Locally, I put them in a new class IMapTest because it didn't feel right
to put them in IMapAsyncTest. The tests are heavily based upon both the
unit tests in place, and the unit tests accompanying the fix for issue
#288 #288.

If this is indeed an issue, and if my fix is correct, I'd be more than
happy to provide it in a pull request if this is easier.

Many thanks,
Raith


Reply to this email directly or view it on GitHubhttps://github.com//issues/472
.

Enes Akar
Hazelcast | Open source in-memory data grid
Mobile: +90.505.394.1668

@PixelRaith

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2013

Hi enesakar, thanks for looking in to this!

I don't know if this is relevant, but the linked issue quotes the part of the documentation that states "Entry is touched if get, put or containsKey is called.", and I would have thought maps would behave consistently in this regard.

Still, you guys know best how things should behave, so I eagerly await the verdict! :)

@enesakar

This comment has been minimized.

Copy link
Member

commented May 7, 2013

Can you send pull request please?

PixelRaith added a commit to PixelRaith/hazelcast that referenced this issue May 7, 2013

Test for Issue hazelcast#472 - IMap containsKey() Does Not Reset Idle…
… Timeout

Add a couple of unit tests to new class IMapTest to verify that the idle timeout is reset when calling both get() and containsKey().

PixelRaith added a commit to PixelRaith/hazelcast that referenced this issue May 7, 2013

Fix for Issue hazelcast#472 - IMap containsKey() Does Not Reset Idle …
…Timeout

ConcurrentMapManager modified to update the record's last accessed time when containsKey() is called.
@PixelRaith

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2013

Done: #475

mdogan added a commit that referenced this issue May 28, 2013

Merge pull request #475 from TheDastardlyRaith/472-imap-containskey-i…
…dle-timeout

Fix for Issue #472 - IMap containsKey() Does Not Reset Idle Timeout
@mdogan

This comment has been minimized.

Copy link
Member

commented May 28, 2013

Fixed by pull request #475

@mdogan mdogan closed this May 28, 2013

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.