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

map.containsKey() does not reset idle time counter #288

Closed
ajitj opened this issue Sep 25, 2012 · 2 comments

Comments

Projects
None yet
2 participants
@ajitj
Copy link

commented Sep 25, 2012

The 2.3 documentation mentions this for map:

        <!--
            Maximum number of seconds for each entry to stay idle in the map. Entries that are
            idle(not touched) for more than <max-idle-seconds> will get
            automatically evicted from the map.
            Entry is touched if get, put or containsKey is called.
            Any integer between 0 and Integer.MAX_VALUE.
            0 means infinite. Default is 0.
        -->
        <max-idle-seconds>0</max-idle-seconds>

Though it says that "Entry is touched if containsKey is called", this doesn't seem to happen. In my usage scenario I need to store the entry if it doesn't exist and reset the idle time counter if it exists. Currently I am doing:

if (map.get(key) == null) {
  map.set(key, value, 0L, TimeUnit.SECONDS);
}

Since I am not making use of the value returned by map.get() in this case map.containsKey() would possibly give me better throughput:

if (!map.containsKey(key)) {
  map.set(key, value, 0L, TimeUnit.SECONDS);
}

If this is a code issue then am I correct in saying that this is a small fix inside CMap.java?

    public boolean containsKey(Request req) {
        Record record = getRecord(req);
        if (record == null) {
            return false;
        } else {
            if (record.isActive() && record.isValid()) {
+               record.setLastAccessed();
                return record.valueCount() > 0;
            }
        }
        return false;
    }

Thanks,
-Ajit

@mdogan

This comment has been minimized.

Copy link
Member

commented Sep 28, 2012

Yes, you're right. Thanks for patch.

@mdogan mdogan closed this in 44e8e5d Sep 28, 2012

@ajitj

This comment has been minimized.

Copy link
Author

commented Sep 28, 2012

Hi mdogan,

A unit test to go with the fix in CMapTest.java:

    @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();
        CMap cmap = new CMap(node.concurrentMapManager, "c:myMap");
        Object key = "1";
        Object value = "istanbul";
        Data dKey = toData(key);
        Data dValue = toData(value);
        cmap.put(newPutRequest(dKey, dValue));
        Record record = cmap.getRecord(dKey);
        long firstAccessTime = record.getLastAccessTime();
        Thread.sleep(10);
        assertTrue(cmap.containsKey(newContainsRequest(dKey, null)));
        record = cmap.getRecord(dKey);
        long secondAccessTime = record.getLastAccessTime();
        assertTrue("record access time should have been updated on containsKey()", secondAccessTime > firstAccessTime);
        node.connectionManager.shutdown();
    }

Thanks,
-Ajit

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.