|
wojciech.kapcia@tigase.net opened 2 years ago
|
|
|
This is same (well, very similar) issue to #issue #142 and boils down to the ID being generated by the database and sometimes not populated on time in the application. @andrzej.wojcik we already discussed it in January and the notion was that "JID is long and uses space" hence using bigint here but... we already have both JID and sha1(JID) columns and indexes so I think we could simply use those instead of autoincremented IDs from the database? What do you think? |
|
In theory, we could, but it would not solve the issue as you have 2 requests (1 to create room and 1 to set vcard) sent by the client 1 by 1 (without waiting for the result). Due to that, there is a race condition on the server side (in the MUC component) as both requests may be (and most likely are) processed at the same time. Now we have NPE/Internal Server Exception, but without this "id" requirement, we would get failure to update entry on the SQL side (as there may be no room config yet, when vcard is being saved). I think we should "serialized" room creation and vcard (iq-set) processing on the server side to be on the same thread (we should queue it properly). |
|
|
Shouldn't those already be processed on same thread and thus be queued? Seems like Though, I'm not sure if storing the room in the repository (i.e. db not |
|
|
I think this AFAIR |
|
|
I think that it's handled differently - direct storing to the repository was disabled (see https://github.com/tigase/tigase-muc/blob/4288f2620647f520c8a33806764a725b5b395e32/src/main/java/tigase/muc/repository/inmemory/InMemoryMucRepository.java#L169-L169 as result of #epicgames-21 ) and it's handled later on: https://github.com/tigase/tigase-muc/blob/4288f2620647f520c8a33806764a725b5b395e32/src/main/java/tigase/muc/modules/PresenceModuleImpl.java#L365-L365 via |
|
|
Actually, |
|
|
If there was an exception it wasn't logged, though it seems lack of |
|
|
But this would not set the |
|
|
Just a wild guess, didn't someone try to use emoji in localpart of the MUC room? |
|
|
(private comment) Longer log: Room data from repo: |
|
|
I'm aware of that but as I said - there wasn't any other error/exception and it would definitely result in WARNING so there should be notification.
From the data - it seems not. |
|
|
I do not see a way to get |
|
|
The weird thing is that creation of a This should leave a trace, but it is the only reason I can think of that would lead to allowing the creation of a room (which would cause an instance of a room with id set to null but creation would fail) but at the same time, it would still have an old room in the database. |
|
|
Another alternative already mentioned before: concurrency / clustering? |
|
|
I do not think that it has anything to do with clustering as, according to timestamps, the room was created hours before its avatar is being set. If so, then it was created and id for it was generated and assigned. Moreover, even avatar was already set for it at least once. Now, after a few hours, in the database, the room is still there but in the memory, we have a room without an avatar and without an id and that can only happen if the room is created (probably created again?). If the room was in fact created again, then it would require our method to create a room to fail to check if the room already exists and this is only possible if the loading of all rooms would fail. Another possibility would be that room was created simultaneously on more than one cluster node, but as single XMPP client connection packets are processed on the local cluster node would require 2 different connections to 2 different cluster nodes to create the same room at the same time... (I do no think this is possible..) |
|
|
OK, so for now we have no idea what happened. From the discussion we had it was agreed to re-work database (#issue #149). Closing for now. |
| Type |
Bug
|
| Priority |
Normal
|
| Assignee | |
| Version |
tigase-server-8.3.0
|
Related
Issue Votes (0)
Watchers (0)