incubator-openmeetings-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maxim Solodovnik <solomax...@gmail.com>
Subject Re: no need to sync on slave if server is null ?!
Date Thu, 27 Dec 2012 02:17:40 GMT
Hello Sebastian,

I catch NPE while debugging :(
Clean just imported DB was used. The NPE was 100% reproducible on my server

I'll doublecheck
On Dec 27, 2012 5:25 AM, "seba.wagner@gmail.com" <seba.wagner@gmail.com>
wrote:

> In fact by reading this method, logically your if-statement should never
> be true.
>
> Cause:
> the method "sendUploadCompletMessageByPublicSID" first tries to get the
> RoomClient with:
> RoomClient currentClient = this.clientListManager
>                                 .getClientByPublicSID(publicSID, false,
> null);
>
> The null at the end means it searches a RoomClient with that publicSID and
> the serverId = null. That means it searches for a Connection/Session
> LOCALLY.
>
> If RoomClient is != null => Then it can directly use the regular sync
> methods. Cause it means that the session is handled on the same server.
> If tRoomClient is NULL, then it means it is handled on a slave server.
> I have impelemented this so that the master can sync the upload complete
> message to the slave.
>
> What you have implemented then at the end with that if-clause, actually
> the implementation logically can never run into that.
> If the serverId == null, the RoomClient would have already be found by the
> first getClientByPublicSID(publicSID, false, null);
>
> From my point of view an Exception should here be thrown and we should
> find out how and when it could happen that the "server" argument is null
> here. Cause actually it would mean a much bigger issue.
>
> Where did you find any kind of issue that makes you think that this
> if-clause is needed?
>
> Sebastian
>
>
>
>
>
>
> 2012/12/27 seba.wagner@gmail.com <seba.wagner@gmail.com>
>
>> Hi Maxim,
>>
>> I found this fix from you:
>>
>> Server s = clientSessionInfo.getServerId() != null ?
>> serverDao.get(clientSessionInfo.getServerId()) : null;
>>             if (s != null) {
>>                 // no need to sync on slave if server is null
>>                 clusterSlaveJob.syncMessageToClientOnSlave(s,
>> clientSessionInfo.getRcl().getPublicSID() , message);
>>             }
>>
>> What should that mean and what enhancements should it bring?
>>
>> Actually if server == null it means that the client is handled on the
>> same server.
>> Basically on a slave ALL sessions have the server == null, because from
>> the perspective of the slave every session is locally. In fact the slave
>> does not even know that he is a slave. He handles every connection as if
>> there is no difference.
>>
>> So why should the slave NOT sync that message ? That makes no sense to me.
>>
>> Server == null is a correct implementation and it should not throw any
>> NullPointerException.
>> It simply means that the Session is local and not on another server.
>> Actually only this kind of session could have a s != null:
>> A session that is synced from the slave to the master. The master would
>> have this session with a Server != null.
>>
>> So your comment does not makes sense to me.
>> Of course slaves do sync messages. On the slave the "server" argument is
>> _always_ null. But of course the slave should still sync that message.
>>
>> Sebastian
>>
>>
>> --
>> Sebastian Wagner
>> https://twitter.com/#!/dead_lock
>> http://www.webbase-design.de
>> http://www.wagner-sebastian.com
>> seba.wagner@gmail.com
>>
>
>
>
> --
> Sebastian Wagner
> https://twitter.com/#!/dead_lock
> http://www.webbase-design.de
> http://www.wagner-sebastian.com
> seba.wagner@gmail.com
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message