Do we really need this lock in RoutingTableImpl.getRoutes()?

Openfire Openfire Dev
1

Openfire acquires a lock with bare JID as key on users session cache in RoutingTableImpl.getRoutes(). Is this really necessary? A comment states " // temporarily block new sessions for this JID". But what would be the negative impact if there is a new session or a session gets removed inside that locked block? This can happen afterwards anyways.

Background is: This is very expensive when Openfire is clustered with hazelcast plugin. E.g. if you send 10k messages to the same bare JID (the method mentioned above gets invoked for each single message) and have clustering enabled, routing is up to 50 times slower than without clustering. Of course locking over cluster nodes is really expensive because it requires network I/O.

Does anyone know what was the intention of this lock?

Many Thanks!

Daniel

2

It’s always a good idea to post a stable link to the code part you are referring to. I guess in this case it’s Openfire/RoutingTableImpl.java at 97297fd69151f85d41e17cb88eabe3c50052d147 · igniterealtime/Openfire · GitHub

If we consult git-blame, we find that the lock was introduced by OF-573: Improve session concurrency and consistency across local and rem… · 9b95747 · igniterealtime/Openfire · GitHu… , sadly the commit message tells nothing about the intention or reasoning behind introducing the lock. But luckily Tom is usually around here and may be able to shed some light into it.

3

Ups, sorry for not posting the code part. Thanks for adding it. Yes this is the part I am referring to.