From b2cb3f6a3a21cdec278a0c433497fe1fcea47953 Mon Sep 17 00:00:00 2001 From: Vedran Pavic Date: Mon, 29 Jan 2018 16:05:41 +0100 Subject: [PATCH] Optimize `HazelcastSessionRepository` write operations Closes gh-983 --- .../hazelcast/HazelcastSessionRepository.java | 89 +++++++++++++++---- .../HazelcastSessionRepositoryTests.java | 80 ++++++++++------- 2 files changed, 117 insertions(+), 52 deletions(-) diff --git a/spring-session/src/main/java/org/springframework/session/hazelcast/HazelcastSessionRepository.java b/spring-session/src/main/java/org/springframework/session/hazelcast/HazelcastSessionRepository.java index 2c551798..cf70bb4c 100644 --- a/spring-session/src/main/java/org/springframework/session/hazelcast/HazelcastSessionRepository.java +++ b/spring-session/src/main/java/org/springframework/session/hazelcast/HazelcastSessionRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,6 +28,8 @@ import javax.annotation.PreDestroy; import com.hazelcast.core.EntryEvent; import com.hazelcast.core.IMap; +import com.hazelcast.map.AbstractEntryProcessor; +import com.hazelcast.map.EntryProcessor; import com.hazelcast.map.listener.EntryAddedListener; import com.hazelcast.map.listener.EntryEvictedListener; import com.hazelcast.map.listener.EntryRemovedListener; @@ -108,8 +110,7 @@ import org.springframework.util.Assert; */ public class HazelcastSessionRepository implements FindByIndexNameSessionRepository, - EntryAddedListener, - EntryEvictedListener, + EntryAddedListener, EntryEvictedListener, EntryRemovedListener { /** @@ -200,11 +201,16 @@ public class HazelcastSessionRepository implements } public void save(HazelcastSession session) { - if (session.isChanged()) { - this.sessions.put(session.getId(), session.getDelegate(), + if (session.isNew) { + this.sessions.set(session.getId(), session.getDelegate(), session.getMaxInactiveIntervalInSeconds(), TimeUnit.SECONDS); - session.markUnchanged(); } + else if (session.changed) { + this.sessions.executeOnKey(session.getId(), + new SessionUpdateEntryProcessor(session.getLastAccessedTime(), + session.getMaxInactiveIntervalInSeconds(), session.delta)); + } + session.clearFlags(); } public HazelcastSession getSession(String id) { @@ -223,13 +229,13 @@ public class HazelcastSessionRepository implements this.sessions.remove(id); } - public Map findByIndexNameAndIndexValue( - String indexName, String indexValue) { + public Map findByIndexNameAndIndexValue(String indexName, + String indexValue) { if (!PRINCIPAL_NAME_INDEX_NAME.equals(indexName)) { return Collections.emptyMap(); } - Collection sessions = this.sessions.values( - Predicates.equal(PRINCIPAL_NAME_ATTRIBUTE, indexValue)); + Collection sessions = this.sessions + .values(Predicates.equal(PRINCIPAL_NAME_ATTRIBUTE, indexValue)); Map sessionMap = new HashMap( sessions.size()); for (MapSession session : sessions) { @@ -270,15 +276,20 @@ public class HazelcastSessionRepository implements final class HazelcastSession implements ExpiringSession { private final MapSession delegate; + + private boolean isNew; + private boolean changed; + private Map delta = new HashMap(); + /** * Creates a new instance ensuring to mark all of the new attributes to be * persisted in the next save operation. */ HazelcastSession() { this(new MapSession()); - this.changed = true; + this.isNew = true; flushImmediateIfNecessary(); } @@ -334,28 +345,28 @@ public class HazelcastSessionRepository implements public void setAttribute(String attributeName, Object attributeValue) { this.delegate.setAttribute(attributeName, attributeValue); + this.delta.put(attributeName, attributeValue); this.changed = true; flushImmediateIfNecessary(); } public void removeAttribute(String attributeName) { this.delegate.removeAttribute(attributeName); + this.delta.put(attributeName, null); this.changed = true; flushImmediateIfNecessary(); } - boolean isChanged() { - return this.changed; - } - - void markUnchanged() { - this.changed = false; - } - MapSession getDelegate() { return this.delegate; } + void clearFlags() { + this.isNew = false; + this.changed = false; + this.delta.clear(); + } + private void flushImmediateIfNecessary() { if (HazelcastSessionRepository.this.hazelcastFlushMode == HazelcastFlushMode.IMMEDIATE) { HazelcastSessionRepository.this.save(this); @@ -364,4 +375,44 @@ public class HazelcastSessionRepository implements } + /** + * Hazelcast {@link EntryProcessor} responsible for handling updates to session. + * + * @since 1.3.2 + * @see #save(HazelcastSession) + */ + private static final class SessionUpdateEntryProcessor + extends AbstractEntryProcessor { + + private final long lastAccessedTime; + + private final int maxInactiveIntervalInSeconds; + + private final Map delta; + + SessionUpdateEntryProcessor(long lastAccessedTime, + int maxInactiveIntervalInSeconds, Map delta) { + this.lastAccessedTime = lastAccessedTime; + this.maxInactiveIntervalInSeconds = maxInactiveIntervalInSeconds; + this.delta = delta; + } + + public Object process(Map.Entry entry) { + MapSession value = entry.getValue(); + value.setLastAccessedTime(this.lastAccessedTime); + value.setMaxInactiveIntervalInSeconds(this.maxInactiveIntervalInSeconds); + for (final Map.Entry attribute : this.delta.entrySet()) { + if (attribute.getValue() != null) { + value.setAttribute(attribute.getKey(), attribute.getValue()); + } + else { + value.removeAttribute(attribute.getKey()); + } + } + entry.setValue(value); + return value; + } + + } + } diff --git a/spring-session/src/test/java/org/springframework/session/hazelcast/HazelcastSessionRepositoryTests.java b/spring-session/src/test/java/org/springframework/session/hazelcast/HazelcastSessionRepositoryTests.java index 3cf8c104..c1de42e3 100644 --- a/spring-session/src/test/java/org/springframework/session/hazelcast/HazelcastSessionRepositoryTests.java +++ b/spring-session/src/test/java/org/springframework/session/hazelcast/HazelcastSessionRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import com.hazelcast.core.IMap; +import com.hazelcast.map.EntryProcessor; import com.hazelcast.query.impl.predicates.EqualPredicate; import org.junit.Before; import org.junit.Rule; @@ -40,6 +41,7 @@ import org.springframework.session.hazelcast.HazelcastSessionRepository.Hazelcas import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.isA; import static org.mockito.Mockito.times; @@ -104,8 +106,8 @@ public class HazelcastSessionRepositoryTests { verifyZeroInteractions(this.sessions); this.repository.save(session); - verify(this.sessions, times(1)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); } @Test @@ -113,8 +115,8 @@ public class HazelcastSessionRepositoryTests { this.repository.setHazelcastFlushMode(HazelcastFlushMode.IMMEDIATE); HazelcastSession session = this.repository.createSession(); - verify(this.sessions, times(1)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); } @Test @@ -124,8 +126,8 @@ public class HazelcastSessionRepositoryTests { verifyZeroInteractions(this.sessions); this.repository.save(session); - verify(this.sessions, times(1)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); } @Test @@ -134,8 +136,10 @@ public class HazelcastSessionRepositoryTests { HazelcastSession session = this.repository.createSession(); session.setAttribute("testName", "testValue"); - verify(this.sessions, times(2)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).executeOnKey(eq(session.getId()), + any(EntryProcessor.class)); this.repository.save(session); verifyZeroInteractions(this.sessions); @@ -148,8 +152,8 @@ public class HazelcastSessionRepositoryTests { verifyZeroInteractions(this.sessions); this.repository.save(session); - verify(this.sessions, times(1)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); } @Test @@ -158,8 +162,10 @@ public class HazelcastSessionRepositoryTests { HazelcastSession session = this.repository.createSession(); session.removeAttribute("testName"); - verify(this.sessions, times(2)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).executeOnKey(eq(session.getId()), + any(EntryProcessor.class)); this.repository.save(session); verifyZeroInteractions(this.sessions); @@ -172,8 +178,8 @@ public class HazelcastSessionRepositoryTests { verifyZeroInteractions(this.sessions); this.repository.save(session); - verify(this.sessions, times(1)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); } @Test @@ -182,8 +188,10 @@ public class HazelcastSessionRepositoryTests { HazelcastSession session = this.repository.createSession(); session.setLastAccessedTime(System.currentTimeMillis()); - verify(this.sessions, times(2)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).executeOnKey(eq(session.getId()), + any(EntryProcessor.class)); this.repository.save(session); verifyZeroInteractions(this.sessions); @@ -196,8 +204,8 @@ public class HazelcastSessionRepositoryTests { verifyZeroInteractions(this.sessions); this.repository.save(session); - verify(this.sessions, times(1)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); } @Test @@ -206,8 +214,10 @@ public class HazelcastSessionRepositoryTests { HazelcastSession session = this.repository.createSession(); session.setMaxInactiveIntervalInSeconds(1); - verify(this.sessions, times(2)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).executeOnKey(eq(session.getId()), + any(EntryProcessor.class)); this.repository.save(session); verifyZeroInteractions(this.sessions); @@ -217,8 +227,8 @@ public class HazelcastSessionRepositoryTests { public void saveUnchangedFlushModeOnSave() { HazelcastSession session = this.repository.createSession(); this.repository.save(session); - verify(this.sessions, times(1)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); this.repository.save(session); verifyZeroInteractions(this.sessions); @@ -229,8 +239,8 @@ public class HazelcastSessionRepositoryTests { this.repository.setHazelcastFlushMode(HazelcastFlushMode.IMMEDIATE); HazelcastSession session = this.repository.createSession(); - verify(this.sessions, times(1)).put(eq(session.getId()), eq(session.getDelegate()), - isA(Long.class), eq(TimeUnit.SECONDS)); + verify(this.sessions, times(1)).set(eq(session.getId()), + eq(session.getDelegate()), isA(Long.class), eq(TimeUnit.SECONDS)); this.repository.save(session); verifyZeroInteractions(this.sessions); @@ -249,8 +259,8 @@ public class HazelcastSessionRepositoryTests { @Test public void getSessionExpired() { MapSession expired = new MapSession(); - expired.setLastAccessedTime(System.currentTimeMillis() - - (MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS * 1000 + 1000)); + expired.setLastAccessedTime(System.currentTimeMillis() + - (MapSession.DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS * 1000 + 1000)); given(this.sessions.get(eq(expired.getId()))).willReturn(expired); HazelcastSession session = this.repository.getSession(expired.getId()); @@ -286,8 +296,8 @@ public class HazelcastSessionRepositoryTests { public void findByIndexNameAndIndexValueUnknownIndexName() { String indexValue = "testIndexValue"; - Map sessions = this.repository.findByIndexNameAndIndexValue( - "testIndexName", indexValue); + Map sessions = this.repository + .findByIndexNameAndIndexValue("testIndexName", indexValue); assertThat(sessions).isEmpty(); verifyZeroInteractions(this.sessions); @@ -297,8 +307,10 @@ public class HazelcastSessionRepositoryTests { public void findByIndexNameAndIndexValuePrincipalIndexNameNotFound() { String principal = "username"; - Map sessions = this.repository.findByIndexNameAndIndexValue( - FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME, principal); + Map sessions = this.repository + .findByIndexNameAndIndexValue( + FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME, + principal); assertThat(sessions).isEmpty(); verify(this.sessions, times(1)).values(isA(EqualPredicate.class)); @@ -318,8 +330,10 @@ public class HazelcastSessionRepositoryTests { saved.add(saved2); given(this.sessions.values(isA(EqualPredicate.class))).willReturn(saved); - Map sessions = this.repository.findByIndexNameAndIndexValue( - FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME, principal); + Map sessions = this.repository + .findByIndexNameAndIndexValue( + FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME, + principal); assertThat(sessions).hasSize(2); verify(this.sessions, times(1)).values(isA(EqualPredicate.class));