done
On Wed, Feb 27, 2013 at 1:16 PM, Maxim Solodovnik <[email protected]>wrote: > 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 > -- WBR Maxim aka solomax
