You are right. I will make it work for the 5.2.0 final.

Andrzej, as you are working on the clustering, could you please share your opinion on this? Can we make the errors work as expected in a clustered environment?

We could make it almost as expected.

There could be at least one issue when error would appear while processing stanza on one of nodes while other nodes containing session for same user processed this stanza fine and stanza was sent with bare jid set as to attribute. Then Tigase XMPP Server could return one or more stanzas with type error if this stanza was sent to bare jid and was processed by more than one node as each node which processed this stanza could and propably will return error stanza.

That could be solved if we used a ring-processing, in theory there should be no concurrent processing for messages on multiple nodes.

What I am more afraid of is an infinite loop of an error packet being sent back and forth between cluster nodes.

I'm not sure if ring processing will fix this as if error would be generated during processing in thread of XMPPProcessor on SM level then it would be out of scope of a ring processing for cluster stanzas.

If we wouldn't generate error stanzas as response for stanza with type error then propably we wouldn't have an issue with infinite loop.

Andrzej Wójcik wrote:

I'm not sure if ring processing will fix this as if error would be generated during processing in thread of XMPPProcessor on SM level then it would be out of scope of a ring processing for cluster stanzas.

Ok, I ma leaving this to you to chose the best solution.

If we wouldn't generate error stanzas as response for stanza with type error then propably we wouldn't have an issue with infinite loop.

You are right but this cannot be guaranteed. I mean, this is a bug in the code if an error is generated as a response to error but it happens. Hopefully our own code was cleaned up and this does not happen (MUC, PubSub?, all SM plugins) but it is possible that a custom code can still do this and we do not want the entire cluster to break because of this.

As for issue with sending error on many times in response to one stanza, then we propably could ignore it as we can't assume that clients to which we forward this stanzas would not return an error many times (one time for each of connected clients/resources)

It is true that we cannot assume that other/custom code will work fine, but we can provide a system property which will allow anyone to enable this if he needs this way of processing and we could mark that in future versions this will be default behavior of Tigase XMPP Server. Am I right?

Andrzej Wójcik wrote:

As for issue with sending error on many times in response to one stanza, then we propably could ignore it as we can't assume that clients to which we forward this stanzas would not return an error many times (one time for each of connected clients/resources)

Makes sense to me.

It is true that we cannot assume that other/custom code will work fine, but we can provide a system property which will allow anyone to enable this if he needs this way of processing and we could mark that in future versions this will be default behavior of Tigase XMPP Server. Am I right?

I am not sure if I understand. Right now we have kind of a protection from responding with an error to error stanza. The protection is in the core API. The Packet class has methods for creating a response for a Packet object, it detects attempt to generate error for an error and throws an exception. However, as far as I know, code in many places create stanzas "manually" without using the API. Is there any other way we can force/protect developers form responding with an error to an error?

I think that when we enable sending errors thru cluster nodes then we will have no possibility to protect developers from making this mistakes. What I suggested is that we could make new config parameter which would enable sending errors but it will also assume that all components are working properly (without any our "protection" mechanisms). But if this parameter would be set to off (default) then we would not propagate errors to other nodes and protection will work. We could also mark somewhere that in version 5.3.0 we would enable error staza delivery to other nodes, which would give time to fix custom code and test it against our new behavior before it will be permanently changed.

Andrzej Wójcik wrote:

I think that when we enable sending errors thru cluster nodes then we will have no possibility to protect developers from making this mistakes. What I suggested is that we could make new config parameter which would enable sending errors but it will also assume that all components are working properly (without any our "protection" mechanisms). But if this parameter would be set to off (default) then we would not propagate errors to other nodes and protection will work. We could also mark somewhere that in version 5.3.0 we would enable error staza delivery to other nodes, which would give time to fix custom code and test it against our new behavior before it will be permanently changed.

Yes, this sounds good to me.

A possible protection from an infinite loop, at least in the cluster code, would be to put some kind of a counter into Packet object, which would be incremented every time a copy of that object is created (for example, when it is forwarded to another node). Once the counter reach some max number, we could simply drop the packet. But this is something for next version.

Ok, this counter could work fine but only if this counter would be forwarded when a response to this stanza is created, while response could be created as Packet.packetInstace(elem), where someone could pass manually created element there, which wouldn't be protected by counter, or by current protection.

That's true, it only solves issue inside our own code, within internal cluster processing where we can make sure the counter is included and incremented.

I added parameter so from now on after setting error-forwarding to forward for SessionManagerClustered component, any error packet will be forwarded to proper cluster node - same as any other packet. If parameter will not be set, then drop will be used as a value, which would force server to not to forward error packets.

Example of entry in etc/init.properties

sess-man/error-forwarding=forward

Rui, please let us know if the solution is satisfactory and if it works now for you as you expected.

Artur, I have not tested this yet, but the solution is satisfactory to me.

Hi Artur/Wojcik,

We are seeing the error messages not being passed to our version 5.2.2 cluster nodes - and we have set:

sess-man/error-forwarding=forward

stanza - not being passed:

2014-11-18 00:49:43.035 [in_23-sess-man] DefaultClusteringStrategyAbstract.processPacket() FINEST: No cluster nodes found for packet forward: from=sess-man@13cn30.lax1.gogii.net, to=null, DATA=You do not have permission to send an SMS message to this country., SIZE=421, XMLNS=jabber:client, PRIORITY=NORMAL, PERMISSION=AUTH, TYPE=error

Todd, it looks to me like your problem is unrelated to this bug. From the attached log, it seems that on your installation, cluster nodes are not connecting to each other. Most likely misconfiguration of the Tigase or network issue.