Skip to content

Commit

Permalink
Issue #5081 - HouseKeeper synchronization (#5099)
Browse files Browse the repository at this point in the history
* Issue #5081 HouseKeeper synchronization

Signed-off-by: Jan Bartel <janb@webtide.com>
  • Loading branch information
janbartel committed Aug 5, 2020
1 parent f416d87 commit 4f57810
Show file tree
Hide file tree
Showing 2 changed files with 203 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public class HouseKeeper extends AbstractLifeCycle
private static final Logger LOG = Log.getLogger("org.eclipse.jetty.server.session");

public static final long DEFAULT_PERIOD_MS = 1000L * 60 * 10;

protected SessionIdManager _sessionIdManager;
protected Scheduler _scheduler;
protected Scheduler.Task _task; //scavenge task
Expand All @@ -62,8 +63,11 @@ public void run()
}
finally
{
if (_scheduler != null && _scheduler.isRunning())
_task = _scheduler.schedule(this, _intervalMs, TimeUnit.MILLISECONDS);
synchronized (HouseKeeper.this)
{
if (_scheduler != null && _scheduler.isRunning())
_task = _scheduler.schedule(this, _intervalMs, TimeUnit.MILLISECONDS);
}
}
}
}
Expand All @@ -75,12 +79,11 @@ public void run()
*/
public void setSessionIdManager(SessionIdManager sessionIdManager)
{
if (isStarted())
throw new IllegalStateException("HouseKeeper started");
_sessionIdManager = sessionIdManager;
}

/**
* @see org.eclipse.jetty.util.component.AbstractLifeCycle#doStart()
*/
@Override
protected void doStart() throws Exception
{
Expand All @@ -92,35 +95,6 @@ protected void doStart() throws Exception
super.doStart();
}

/**
* Get a scheduler. First try a common scheduler, failing that
* create our own.
*
* @throws Exception when the scheduler cannot be started
*/
protected void findScheduler() throws Exception
{
if (_scheduler == null)
{
if (_sessionIdManager instanceof DefaultSessionIdManager)
{
//try and use a common scheduler, fallback to own
_scheduler = ((DefaultSessionIdManager)_sessionIdManager).getServer().getBean(Scheduler.class);
}

if (_scheduler == null)
{
_scheduler = new ScheduledExecutorScheduler(String.format("Session-HouseKeeper-%x", hashCode()), false);
_ownScheduler = true;
_scheduler.start();
if (LOG.isDebugEnabled())
LOG.debug("Using own scheduler for scavenging");
}
else if (!_scheduler.isStarted())
throw new IllegalStateException("Shared scheduler not started");
}
}

/**
* If scavenging is not scheduled, schedule it.
*
Expand All @@ -130,16 +104,33 @@ protected void startScavenging() throws Exception
{
synchronized (this)
{
if (_scheduler != null)
if (_scheduler == null)
{
//cancel any previous task
if (_task != null)
_task.cancel();
if (_runner == null)
_runner = new Runner();
LOG.info("{} Scavenging every {}ms", _sessionIdManager.getWorkerName(), _intervalMs);
_task = _scheduler.schedule(_runner, _intervalMs, TimeUnit.MILLISECONDS);
if (_sessionIdManager instanceof DefaultSessionIdManager)
{
//try and use a common scheduler, fallback to own
_scheduler = ((DefaultSessionIdManager)_sessionIdManager).getServer().getBean(Scheduler.class);
}

if (_scheduler == null)
{
_scheduler = new ScheduledExecutorScheduler(String.format("Session-HouseKeeper-%x", hashCode()), false);
_ownScheduler = true;
_scheduler.start();
if (LOG.isDebugEnabled())
LOG.debug("Using own scheduler for scavenging");
}
else if (!_scheduler.isStarted())
throw new IllegalStateException("Shared scheduler not started");
}

//cancel any previous task
if (_task != null)
_task.cancel();
if (_runner == null)
_runner = new Runner();
LOG.info("{} Scavenging every {}ms", _sessionIdManager.getWorkerName(), _intervalMs);
_task = _scheduler.schedule(_runner, _intervalMs, TimeUnit.MILLISECONDS);
}
}

Expand All @@ -164,13 +155,10 @@ protected void stopScavenging() throws Exception
_scheduler.stop();
_scheduler = null;
}
_runner = null;
}
_runner = null;
}

/**
* @see org.eclipse.jetty.util.component.AbstractLifeCycle#doStop()
*/
@Override
protected void doStop() throws Exception
{
Expand All @@ -190,37 +178,39 @@ protected void doStop() throws Exception
*/
public void setIntervalSec(long sec) throws Exception
{
if (isStarted() || isStarting())
synchronized (this)
{
if (sec <= 0)
{
_intervalMs = 0L;
LOG.info("{} Scavenging disabled", _sessionIdManager.getWorkerName());
stopScavenging();
}
else
if (isStarted() || isStarting())
{
if (sec < 10)
LOG.warn("{} Short interval of {}sec for session scavenging.", _sessionIdManager.getWorkerName(), sec);
if (sec <= 0)
{
_intervalMs = 0L;
LOG.info("{} Scavenging disabled", _sessionIdManager.getWorkerName());
stopScavenging();
}
else
{
if (sec < 10)
LOG.warn("{} Short interval of {}sec for session scavenging.", _sessionIdManager.getWorkerName(), sec);

_intervalMs = sec * 1000L;
_intervalMs = sec * 1000L;

//add a bit of variability into the scavenge time so that not all
//nodes with the same scavenge interval sync up
long tenPercent = _intervalMs / 10;
if ((System.currentTimeMillis() % 2) == 0)
_intervalMs += tenPercent;
//add a bit of variability into the scavenge time so that not all
//nodes with the same scavenge interval sync up
long tenPercent = _intervalMs / 10;
if ((System.currentTimeMillis() % 2) == 0)
_intervalMs += tenPercent;

if (isStarting() || isStarted())
{
findScheduler();
startScavenging();
if (isStarting() || isStarted())
{
startScavenging();
}
}
}
}
else
{
_intervalMs = sec * 1000L;
else
{
_intervalMs = sec * 1000L;
}
}
}

Expand All @@ -232,7 +222,10 @@ public void setIntervalSec(long sec) throws Exception
@ManagedAttribute(value = "secs between scavenge cycles", readonly = true)
public long getIntervalSec()
{
return _intervalMs / 1000;
synchronized (this)
{
return _intervalMs / 1000;
}
}

/**
Expand Down Expand Up @@ -264,12 +257,12 @@ public void scavenge()
}
}

/**
* @see java.lang.Object#toString()
*/
@Override
public String toString()
{
return super.toString() + "[interval=" + _intervalMs + ", ownscheduler=" + _ownScheduler + "]";
synchronized (this)
{
return super.toString() + "[interval=" + _intervalMs + ", ownscheduler=" + _ownScheduler + "]";
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
//
// ========================================================================
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//

package org.eclipse.jetty.server.session;

import java.util.Collections;
import java.util.Set;

import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.util.thread.Scheduler;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* HouseKeeperTest
*/
public class HouseKeeperTest
{
public class TestHouseKeeper extends HouseKeeper
{
public Scheduler getScheduler()
{
return _scheduler;
}

public Scheduler.Task getTask()
{
return _task;
}

public Runner getRunner()
{
return _runner;
}

public boolean isOwnScheduler()
{
return _ownScheduler;
}
}

public class TestSessionIdManager extends DefaultSessionIdManager
{
public TestSessionIdManager(Server server)
{
super(server);
}

@Override
public Set<SessionHandler> getSessionHandlers()
{
return Collections.singleton(new SessionHandler());
}
}

@Test
public void testHouseKeeper() throws Exception
{
HouseKeeper t = new TestHouseKeeper();
assertThrows(IllegalStateException.class, () -> t.start());

TestHouseKeeper hk = new TestHouseKeeper();
hk.setSessionIdManager(new TestSessionIdManager(new Server()));
hk.setIntervalSec(-1);
hk.start(); //no scavenging

//check that the housekeeper isn't running
assertNull(hk.getRunner());
assertNull(hk.getTask());
assertNull(hk.getScheduler());
assertFalse(hk.isOwnScheduler());
hk.stop();
assertNull(hk.getRunner());
assertNull(hk.getTask());
assertNull(hk.getScheduler());
assertFalse(hk.isOwnScheduler());

//set the interval but don't start it
hk.setIntervalSec(10000);
assertNull(hk.getRunner());
assertNull(hk.getTask());
assertNull(hk.getScheduler());
assertFalse(hk.isOwnScheduler());

//now start it
hk.start();
assertNotNull(hk.getRunner());
assertNotNull(hk.getTask());
assertNotNull(hk.getScheduler());
assertTrue(hk.isOwnScheduler());

//stop it
hk.stop();
assertNull(hk.getRunner());
assertNull(hk.getTask());
assertNull(hk.getScheduler());
assertFalse(hk.isOwnScheduler());

//start it, but set a different interval after start
hk.start();
Scheduler.Task oldTask = hk.getTask();
hk.setIntervalSec(50000);
assertTrue(hk.getIntervalSec() >= 50000);
assertNotNull(hk.getRunner());
assertNotNull(hk.getTask());
//Note: it would be nice to test if the old task was
//cancelled, but the Scheduler.Task interface does not
//provide that functionality.
assertNotSame(oldTask, hk.getTask());
assertNotNull(hk.getScheduler());
assertTrue(hk.isOwnScheduler());
}
}

0 comments on commit 4f57810

Please sign in to comment.