Hi Maxim, I don't understand your changes to the HashMapStore.
Why is there a reference to the ServerUtil in the HashMapStore at all? The HashMapStore should only be used if _no_ server is configured at all, no cluster, zero :) So actually getCurrentServer should not be used in the case where you are using memory based store (aka "HashMapStore"). And in that sense: public List<Long> getRoomsIdsByServer(Server server) simply should always return the current list of all roomIds, not a subset of it. In case of HashMapStore there is no cluster or other server involved, so its logically impossible that this function will be called with any server. But maybe I don't understand the entire scope of the changes that you committed. Sebastian 2013/2/27 <[email protected]> > Author: solomax > Date: Wed Feb 27 05:28:29 2013 > New Revision: 1450608 > > URL: http://svn.apache.org/r1450608 > Log: > SIP: additional NPE check is added > Cluster: getServerForSession is refactored to work as expected with no > overhead > > Modified: > > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/RoomDao.java > > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/SipDao.java > > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/basic/Server.java > > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/room/Room.java > > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/remote/ConferenceService.java > > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/SessionManager.java > > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/store/HashMapStore.java > > Modified: > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/RoomDao.java > URL: > http://svn.apache.org/viewvc/openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/RoomDao.java?rev=1450608&r1=1450607&r2=1450608&view=diff > > ============================================================================== > --- > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/RoomDao.java > (original) > +++ > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/RoomDao.java > Wed Feb 27 05:28:29 2013 > @@ -75,8 +75,7 @@ public class RoomDao implements IDataPro > } > > public List<Room> getPublicRooms() { > - return em.createNamedQuery("getPublicRoomsOrdered", > Room.class) > - .getResultList(); > + return em.createNamedQuery("getPublicRoomsOrdered", > Room.class).getResultList(); > } > > public List<Long> getSipRooms(List<Long> ids) { > @@ -96,6 +95,11 @@ public class RoomDao implements IDataPro > return q.getResultList(); > } > > + public Long getRoomsCapacityByIds(List<Long> ids) { > + return ids == null || ids.isEmpty() ? 0 > + : em.createNamedQuery("getRoomsCapacityByIds", > Long.class).setParameter("ids", ids).getSingleResult(); > + } > + > private boolean isSipEnabled() { > return "yes".equals(cfgDao.getConfValue("red5sip.enable", > String.class, "no")); > } > > Modified: > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/SipDao.java > URL: > http://svn.apache.org/viewvc/openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/SipDao.java?rev=1450608&r1=1450607&r2=1450608&view=diff > > ============================================================================== > --- > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/SipDao.java > (original) > +++ > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/data/conference/dao/SipDao.java > Wed Feb 27 05:28:29 2013 > @@ -59,6 +59,10 @@ public class SipDao { > } > > private ManagerResponse exec(ManagerAction action) { > + if (connection == null) { > + log.warn("There is no Asterisk configured"); > + return null; > + } > try { > connection.login(); > ManagerResponse r = connection.sendAction(action); > @@ -73,6 +77,10 @@ public class SipDao { > } > > private ResponseEvents execEvent(EventGeneratingAction action) { > + if (connection == null) { > + log.warn("There is no Asterisk configured"); > + return null; > + } > try { > connection.login("on"); > ResponseEvents r = > connection.sendEventGeneratingAction(action); > > Modified: > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/basic/Server.java > URL: > http://svn.apache.org/viewvc/openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/basic/Server.java?rev=1450608&r1=1450607&r2=1450608&view=diff > > ============================================================================== > --- > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/basic/Server.java > (original) > +++ > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/basic/Server.java > Wed Feb 27 05:28:29 2013 > @@ -258,6 +258,27 @@ public class Server implements Serializa > } > > @Override > + public boolean equals(Object obj) { > + if (obj == null) { > + return false; > + } > + if (!(obj instanceof Server)) { > + return false; > + } else { > + Server s = (Server)obj; > + return s.id == id && ((s.name != null && > s.name.equals(name)) || (s.name == null && s.name == name)); > + } > + } > + > + @Override > + public int hashCode() { > + final int prime = 31; > + int result = 1; > + result = prime * result + (int)id + (name == null ? 0 : > name.hashCode()); > + return result; > + } > + > + @Override > public String toString() { > return "Server [id=" + id + ", name=" + name + ", > address=" + address > + ", port=" + port + ", user=" + user + ", > pass=" + pass > > Modified: > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/room/Room.java > URL: > http://svn.apache.org/viewvc/openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/room/Room.java?rev=1450608&r1=1450607&r2=1450608&view=diff > > ============================================================================== > --- > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/room/Room.java > (original) > +++ > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/persistence/beans/room/Room.java > Wed Feb 27 05:28:29 2013 > @@ -72,7 +72,8 @@ import org.simpleframework.xml.Root; > @NamedQuery(name = "getRoomById", query = "SELECT r FROM Room r > WHERE r.deleted = false AND r.rooms_id = :id"), > @NamedQuery(name = "getSipRoomIdsByIds", query = "SELECT > r.rooms_id FROM Room r WHERE r.deleted = false AND r.sipEnabled = true AND > r.rooms_id IN :ids"), > @NamedQuery(name = "countRooms", query = "SELECT COUNT(r) FROM > Room r WHERE r.deleted = false"), > - @NamedQuery(name = "getBackupRooms", query = "SELECT r FROM Room r > LEFT JOIN FETCH r.moderators WHERE r.deleted = false ") > + @NamedQuery(name = "getBackupRooms", query = "SELECT r FROM Room r > LEFT JOIN FETCH r.moderators WHERE r.deleted = false "), > + @NamedQuery(name = "getRoomsCapacityByIds", query = "SELECT > SUM(r.numberOfPartizipants) FROM Room r WHERE r.deleted = false AND > r.rooms_id IN :ids") > }) > @Table(name = "room") > @Root(name = "room") > > Modified: > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/remote/ConferenceService.java > URL: > http://svn.apache.org/viewvc/openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/remote/ConferenceService.java?rev=1450608&r1=1450607&r2=1450608&view=diff > > ============================================================================== > --- > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/remote/ConferenceService.java > (original) > +++ > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/remote/ConferenceService.java > Wed Feb 27 05:28:29 2013 > @@ -19,16 +19,12 @@ > package org.apache.openmeetings.remote; > > import java.util.ArrayList; > -import java.util.Collections; > -import java.util.Comparator; > import java.util.Date; > import java.util.HashMap; > import java.util.Iterator; > import java.util.LinkedHashMap; > -import java.util.LinkedList; > import java.util.List; > import java.util.Map; > -import java.util.Map.Entry; > import java.util.TimeZone; > > import org.apache.openmeetings.OpenmeetingsVariables; > @@ -775,98 +771,29 @@ public class ConferenceService { > if (authLevelUtil.checkUserLevel(user_level)) { > List<Server> serverList = > serverDao.getActiveServers(); > > - // if there is no cluster set up, just redirect to > the current one > - if (serverList.size() == 0) { > - return null; > - } > - > - // if the room is already opened on a server, > redirect the user to that one, > - // we do that in two loop's because there is no > query involved here, > - // only the first user that enters the conference > room needs to be adjusted > - // to that server that has the less maxUser count > in its rooms currently. > - // But if the room is already opened, then the > maxUser is no more relevant, > - // the user will be just redirected to the same > server > - > + long minimum = -1; > + Server result = null; > + HashMap<Server, List<Long>> activeRoomsMap = new > HashMap<Server, List<Long>>(); > for (Server server : serverList) { > - for (Long activeRoomId : > sessionManager.getActiveRoomIdsByServer(server)) { > - if (activeRoomId.equals(roomId)) { > - return new > ServerDTO(server); > - } > + List<Long> roomIds = > sessionManager.getActiveRoomIdsByServer(server); > + if (roomIds.contains(roomId)) { > + // if the room is already opened > on a server, redirect the user to that one, > + log.debug("Room is already opened > on a server " + server.getAddress()); > + return new ServerDTO(server); > } > + activeRoomsMap.put(server, roomIds); > } > - > - // the room is not opened on any server yet, its > the first user, get the maxUser > - // per room > - > - // TODO / FIXME: Get room's maxUser in a single > query instead a query for each room > - final Map<Server, List<Room>> serverRoomMap = new > HashMap<Server, List<Room>>(); > - // Slave/Server rooms > - for (Server server : serverList) { > - List<Room> roomList = new > ArrayList<Room>(); > - for (Long activeRoomId : > sessionManager.getActiveRoomIdsByServer(server)) { > - // FIXME / TODO: This is the > single query to get the room by its id > - > roomList.add(roomDao.get(activeRoomId)); > + for (Server server : activeRoomsMap.keySet()) { > + List<Long> roomIds = > activeRoomsMap.get(server); > + Long capacity = > roomDao.getRoomsCapacityByIds(roomIds); > + if (minimum < 0 || capacity < minimum) { > + minimum = capacity; > + result = server; > } > - serverRoomMap.put(server, roomList); > - } > - > - // calc server with lowest max users > - List<Server> list = new LinkedList<Server>(); > - list.addAll(serverRoomMap.keySet()); > - Collections.sort(list, new Comparator<Server>() { > - public int compare(Server s1, Server s2) { > - int maxUsersInRoomS1 = 0; > - log.debug("serverRoomMap.get(s1) > SIZE " + serverRoomMap.get(s1).size()); > - for (Room room : > serverRoomMap.get(s1)) { > - log.debug("s1 room: " + > room); > - maxUsersInRoomS1 += > room.getNumberOfPartizipants(); > - } > - int maxUsersInRoomS2 = 0; > - log.debug("serverRoomMap.get(s2) > SIZE " + serverRoomMap.get(s2).size()); > - for (Room room : > serverRoomMap.get(s2)) { > - log.debug("s2 room: " + > room); > - maxUsersInRoomS2 += > room.getNumberOfPartizipants(); > - } > - > - return maxUsersInRoomS1 - > maxUsersInRoomS2; > - } > - }); > - > - LinkedHashMap<Server, List<Room>> > serverRoomMapOrdered = new LinkedHashMap<Server, List<Room>>(); > - for (Server server : list) { > - serverRoomMapOrdered.put(server, > serverRoomMap.get(server)); > + log.debug("Checking server: " + server + " > Number of rooms " + roomIds.size() + " RoomIds: " > + + roomIds + " max(Sum): " > + capacity); > } > - > - if (log.isDebugEnabled()) { > - log.debug("Resulting order: "); > - for (Entry<Server, List<Room>> entry : > serverRoomMapOrdered.entrySet()) { > - int maxUsersInRoom = 0; > - for (Room room : entry.getValue()) > { > - maxUsersInRoom += > room.getNumberOfPartizipants(); > - } > - > - String roomids = ""; > - for (Room r : entry.getValue()) { > - roomids += " " + > r.getRooms_id(); > - } > - > - log.debug("entry " + > entry.getKey() + " Number of rooms " + entry.getValue().size() + " RoomIds: > " > - + roomids + " > max(Sum): " + maxUsersInRoom); > - } > - > - } > - > - log.debug("Resulting Server"); > - > - Server s = > serverRoomMapOrdered.entrySet().iterator().next().getKey(); > - > - if (s == null) { > - return null; > - } > - > - // Somehow this object here cannot be serialized > cause its abused by OpenJPA > - // so get a fresh copy from the entity manager and > return that > - return new ServerDTO(s); > + return result == null ? null : new > ServerDTO(result); > } > > log.error("Could not get server for cluster session"); > > Modified: > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/SessionManager.java > URL: > http://svn.apache.org/viewvc/openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/SessionManager.java?rev=1450608&r1=1450607&r2=1450608&view=diff > > ============================================================================== > --- > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/SessionManager.java > (original) > +++ > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/SessionManager.java > Wed Feb 27 05:28:29 2013 > @@ -432,10 +432,7 @@ public class SessionManager implements I > } > > public List<Long> getActiveRoomIdsByServer(Server server) { > - if (server == null) { > - server = serverUtil.getCurrentServer(); > - } > - return sessionManager.getActiveRoomIdsByServer(server); > + return sessionManager.getActiveRoomIdsByServer(server == > null ? serverUtil.getCurrentServer() : server); > } > > public String getSessionStatistics() { > > Modified: > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/store/HashMapStore.java > URL: > http://svn.apache.org/viewvc/openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/store/HashMapStore.java?rev=1450608&r1=1450607&r2=1450608&view=diff > > ============================================================================== > --- > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/store/HashMapStore.java > (original) > +++ > openmeetings/trunk/singlewebapp/src/org/apache/openmeetings/session/store/HashMapStore.java > Wed Feb 27 05:28:29 2013 > @@ -29,8 +29,10 @@ import java.util.Map; > import org.apache.openmeetings.OpenmeetingsVariables; > import org.apache.openmeetings.persistence.beans.basic.Server; > import org.apache.openmeetings.persistence.beans.room.Client; > +import org.apache.openmeetings.session.ServerUtil; > import org.red5.logging.Red5LoggerFactory; > import org.slf4j.Logger; > +import org.springframework.beans.factory.annotation.Autowired; > > /** > * Stores the session in the memory. > @@ -48,9 +50,10 @@ import org.slf4j.Logger; > * > */ > public class HashMapStore implements IClientPersistenceStore { > - > protected static final Logger log = Red5LoggerFactory.getLogger( > HashMapStore.class, > OpenmeetingsVariables.webAppRootKey); > + @Autowired > + private ServerUtil serverUtil; > > > private LinkedHashMap<String, Client> clientsByStreamId = new > LinkedHashMap<String, Client>(); > @@ -179,7 +182,7 @@ public class HashMapStore implements ICl > HashSet<Long> rooms = new HashSet<Long>(); > for (Client cl : clientsByStreamId.values()) { > Long roomId = cl.getRoom_id(); > - Server s = cl.getServer(); > + Server s = cl.getServer() == null ? > serverUtil.getCurrentServer() : cl.getServer(); > if ((server == null && s != null) || (server != > null && s == null)) { > continue; > } > > > -- Sebastian Wagner https://twitter.com/#!/dead_lock http://www.webbase-design.de http://www.wagner-sebastian.com [email protected]
