validatorPingPeriod/Timeout etc. not properly passed from me

[gsanderson]

Again this is with JBoss 4.3.0 CP03

I tried additionally using remoting 2.2.2.SP11 which fixes a related bug in the remoting layer.

The basic problem appears to be with JMSRemotingConnection.start()

It creates an underlying Client passing the URI and a Map of config parameters. The config parameters map is populated explicitly from the URI (but only two parameters : enableLease and clientMaxPoolSize)

The remoting Client when creating a ConnectionValidator looks in the config Map (not the URI) for the validatorPingPeriod/Timeout etc. and since they're not there ends up using the defaults.

It appears the intention of the design is that the caller should populate the Map with all relevant parameters rather than having the remoting layer automatically pull them from the URI, but it is hard to be 100% sure.

In any case, right now this prevents you from override the default ping period and timeout via your remoting-bisocket-service.xml (even though these do make it all the way to the client and indeed the URI passed down to the remoting layer).

I checked the tip of 1.4.0, 1.4.1 and 1.4.2 and as far as I can tell none of them pass this info down.

[gsanderson]

the title which was truncated was supposed to "... passed from messaging to remoting layers".

I locally patched JMSRemotingConnection to copy the validatorPingTimeout and validatorPingPeriod from the URI into the config map, and everything worked as expected (i.e. the remoting-bisocket-service.xml values from the server were used on the client for the connection validation).

[timfox]

Howard- could you verify this and whether or not it is fixed in the latest remoting release?

[gaohoward]

sure I'll verify.

[gaohoward]

This should be a JBM issue. The client configuration parameters is not passed to the ConnectionValidator in jboss remoting, which performs the client ping operation.

In JMSRemotingConnection.addConnectionListener() and addPlainConnectionListener(), it uses client.addConnectionListener(listener), which doesn't pass in the config. Rather it should use the client.addConnectionListener(listener, metadata) so the configuration can get populated into remoting layer.

I'll create a JIRA issue for that.

Thanks

[ron_sigal]

"gsanderson" wrote:

but it is hard to be 100% sure.

I know what you mean. :)

"gsanderson" wrote:

It appears the intention of the design is that the caller should populate the Map with all relevant parameters rather than having the remoting layer automatically pull them from the URI

Actually, org.jboss.remoting.Client passes a reference to itself to the ConnectionValidator, which then has access to the Client's configuration map, which includes the InvokerLocator (URI) parameters. ConnectionValidator will take parameters from the Client's configuration map and from the metadata map passed to Client.addConnectionListener(), with the later taking precedence. The problem prior to JBREM-1069 "Make ConnectorValidator configure ping period correctly" (https://jira.jboss.org/jira/browse/JBREM-1069) was that the single argument version of Client.addConnectionListener() put the default value for "validatorPingPeriod" in the metadata map, which meant that the default value would override the value in the InvokerLocator. With the fix to JBREM-1069, the value of "validatorPingPeriod" in the InvokerLocator should take effect. The value of "validatorPingTimeout" should already be taken from the InvokerLocator.

"gaohoward" wrote:

This should be a JBM issue. The client configuration parameters is not passed to the ConnectionValidator in jboss remoting, which performs the client ping operation.

As much as I'm happy to put the blame on JBM, I think it's a Remoting issue. This might be a first. Ask Tim. :)

[gsanderson]

Actually, org.jboss.remoting.Client passes a reference to itself to the ConnectionValidator, which then has access to the Client's configuration map, which includes the InvokerLocator (URI) parameters

The problem (or at least the one I fixed locally) is that the client configuration map is constructed from scratch by JMSRemotingConnection and doesn't contain the validatorPingTimeout or validatorPingPeriod from the URI at all.

https://jira.jboss.org/jira/browse/JBREM-1069 is required to avoid the remoting layer overwriting it once it actually makes it in ;-)

[gsanderson]

actually now I'm confused... I still don't know who is responsible for the client configuration map contents.

I'll let you guys who know what you're talking about figure it out ;-)

[ron_sigal]

"gsanderson" wrote:

actually now I'm confused...

With good reason. I lied to you.

"gsanderson" wrote:

org.jboss.remoting.Client passes a reference to itself to the ConnectionValidator, which then has access to the Client's configuration map, which includes the InvokerLocator (URI) parameters.

That, the part about the InvokerLocator parameters, just isn't true. I was lost in my own blind spot. I've created JBREM-1082 "Allow ConnectionValidator to access InvokerLocator parameters" (https://jira.jboss.org/jira/browse/JBREM-1082) to resolve the problem.

In the meantime, a workaround available to JBM, if they haven't finished their current CP release, would be to go ahead with JBMESSAGING-1482 "Client remoting configuration not properly passed to jboss remoting" (https://jira.jboss.org/jira/browse/JBMESSAGING-1482).

Apologies all around.

-Ron

[gaohoward]

If JBREM-1082 is done, the problem should be solved. Otherwise we need JBM-1482 done.

As JBM's next cp is using JBR sp11, so we need 1482 be fixed.

[ron_sigal]

"gaohoward" wrote:

As JBM's next cp is using JBR sp11, so we need 1482 be fixed.

Howard, will JBMESSAGING-1482 make it into the next EAP CP release?

[gaohoward]

Yes Ron. For EAP cp04.

[rkonjeti]

I see many issues/bugs related to default pingPeriod and pingTimeout very low. There are bugs in JBossMessaging, etc that uses BiSocket.

If so, why dont we just change the defaults for ConnectionValidator to be 10 seconds and 5 seconds to begin with...Ofcourse, allow anybody to override it, if they need to. But, the defaults should be meaningful to begin with.

[timfox]

It's already been done