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

Data loss in webfilter (hazelcast 3.2.2) #2746

Closed
dfex55 opened this issue Jun 17, 2014 · 3 comments
Closed

Data loss in webfilter (hazelcast 3.2.2) #2746

dfex55 opened this issue Jun 17, 2014 · 3 comments

Comments

@dfex55
Copy link

dfex55 commented Jun 17, 2014

Preconditions:

  • webfilter with deferred write enabled
  • valid hazelcast session id
  • session is distributed
  • value in session for key "data" is "old"
  • webfilter node does not have a local version
    -> happens on failover and restart

Procedure to reproduce data loss:

  • call getSession()
  • call session.setAttribute("data", "new")
  • call session.getAttribute("data") will return "old"

-> data loss occured, should return "new"

What happens:

The call of getSession

public HazelcastHttpSession getSession(final boolean create){
        ...
        hazelcastSession = fetchHazelcastSession();
        ...
        if (deferredWrite) {
             prepareReloadingSession(hazelcastSession);
        }
        ...
 }

calls fetchHazelcastSession()

public HazelcastHttpSession fetchHazelcastSession() {
    if (requestedSessionId == null) {
        requestedSessionId = getSessionCookie(this);
    }
    if (requestedSessionId == null) {
        requestedSessionId = getParameter(HAZELCAST_SESSION_COOKIE_NAME);
    }

    if (requestedSessionId != null) {
        hazelcastSession = getSessionWithId(requestedSessionId);
        if (hazelcastSession == null) {
            final Boolean existing = (Boolean) getClusterMap().get(requestedSessionId);
            if (existing != null && existing) {
                // we already have the session in the cluster loading it...
                hazelcastSession = createNewSession(RequestWrapper.this, requestedSessionId);
            }
        }
    }

    return hazelcastSession;
}

Call of getSessionWithId(..) returns null, because MAP_SESSIONS is empty or does not contain the map, see:

private HazelcastHttpSession getSessionWithId(final String sessionId) {
    HazelcastHttpSession session = MAP_SESSIONS.get(sessionId);
    if (session != null && !session.isValid()) {
        destroySession(session, true);
        session = null;
    }
    return session;
}

Continue in fetchHazelcastSession():
Because of the valid session id and distributed content the boolean "existing" is true and createNewSession(…) is called. In createNewSession the method loadHazelcastSession() is called which loads the distributed map and copies its contents to the local cache.

private HazelcastHttpSession createNewSession(RequestWrapper requestWrapper, String existingSessionId) {
…
    if (deferredWrite) {
        loadHazelcastSession(hazelcastSession);
    }
    return hazelcastSession;
}

After copying all entries to the local cache, prepareReloadingSession(..) is called (back in getSession(..)) and all local cache entries will be set to reload = true.

private void prepareReloadingSession(HazelcastHttpSession hazelcastSession) {
    if (deferredWrite && hazelcastSession != null) {
        Map<String, LocalCacheEntry> cache = hazelcastSession.localCache;
        for (LocalCacheEntry cacheEntry : cache.values()) {
            cacheEntry.reload = true;
        }
    }
}

From here, every setAttribute("data", xxx) call updates the local cache, but if you call getAttribute("data") the value will be fetched again from the distributed map because reload is set to true.

Possible solutions are:

  1. Set reload to false in setAttribute
  2. Don't set reload to true directly after fetching values from cluster

To me, both has to be implemented.

@noctarius
Copy link
Contributor

Wow amazing report, thanks :)

@noctarius noctarius added this to the 3.2.3 milestone Jun 17, 2014
@enesakar enesakar modified the milestones: 3.2.4, 3.2.3 Jun 18, 2014
@mesutcelik
Copy link

Hi @dfex55 ,

The code snippet you used is not 3.2.2(mentioned in the issue title) but our master branch. is it the case?

I have also simply added following test case and did not see the issue.
Can you please try the same locally?

@Test
public void issue2746() throws Exception{
    IMap<String, Object> map = hz.getMap("default");

    CookieStore cookieStore = new BasicCookieStore();
    executeRequest("write", serverPort2, cookieStore);
    server1.stop();
    server1.start();
    executeRequest("update", serverPort1, cookieStore);

    String value = executeRequest("read", serverPort1, cookieStore);
    assertEquals("value-updated", value);

    value = executeRequest("read", serverPort2, cookieStore);
    assertEquals("value-updated", value);

}

@dfex55
Copy link
Author

dfex55 commented Jul 7, 2014

Hi @mesutcelik ,
yes you are right. The snippets were taken from the master branch, but code-flow is the same at the v3.2.2 tag.

To reproduce the bug, you have to slightly modify your test.

Add an action to TestServlet:

} else if (req.getRequestURI().endsWith("update-and-read-for-issue2746")) {
        session.setAttribute("key", "value-updated");
        Object value = session.getAttribute("key");
        resp.getWriter().write(value == null ? "null" : value.toString());

then modify your test:

    String value = executeRequest("update-and-read-for-issue2746", serverPort1, cookieStore);
    assertEquals("value-updated", value);

The bug happens within the same request.

mesutcelik pushed a commit to mesutcelik/hazelcast that referenced this issue Jul 7, 2014
mesutcelik pushed a commit to mesutcelik/hazelcast that referenced this issue Jul 7, 2014
hasancelik pushed a commit to hasancelik/hazelcast that referenced this issue Jul 8, 2014
ChristerF pushed a commit to ChristerF/hazelcast that referenced this issue Aug 11, 2014
frant-hartm pushed a commit that referenced this issue Mar 26, 2021
- we must check `hasHadoopPrefix` first before
Paths.get(path).isAbsolute, because path such as `hdfs://something`
fails on windows due to the `:`

- there should be only one `assumeThatNoWindowsOS` to comment out if we
want to test on Windows

* Fix javadoc
devOpsHazelcast pushed a commit that referenced this issue Jul 25, 2024
GitOrigin-RevId: 296829b3a372c02fb12b06dc42a0f93195e8af47
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants