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 16:28:49 GMT
the steps are very simple:

0) no servers are configured
1) select system restore in SWF admin
2) select backup
3) click "upload"
Result: Server is NULL, NPE without a fix (seems it affects nothing)


On Thu, Dec 27, 2012 at 9:18 AM, seba.wagner@gmail.com <
seba.wagner@gmail.com> wrote:

> Please give me the scenario to reproduce it when you have one.
>
> Thanks!
> Sebastian
>
>
> 2012/12/27 Maxim Solodovnik <solomax666@gmail.com>
>
>> 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
>>>
>>
>
>
> --
> Sebastian Wagner
> https://twitter.com/#!/dead_lock
> http://www.webbase-design.de
> http://www.wagner-sebastian.com
> seba.wagner@gmail.com
>



-- 
WBR
Maxim aka solomax

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