Hello Sebastian, in your instructions to Denis you haven't mention MapStore should be switched So I have tested it with HashMapStore ...
I'll revert changes to HashMapStore On Wed, Feb 27, 2013 at 1:10 PM, [email protected] < [email protected]> wrote: > 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] > -- WBR Maxim aka solomax
