Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AS7-5492. Make JNDI listBindings() work with multiple NamingStores. #3016

Closed
wants to merge 1 commit into from

Conversation

doctau
Copy link

@doctau doctau commented Sep 6, 2012

This is a small fix for AS7-5492, by making listChildren() return items from the ServiceRegistry too.

Potential performance concerns:

  1. It iterates through the entire contents of the registry. I don't see a way of fixing that without adding additional ServiceRegistry API to either get partial contents or to notify the store of changes.
  2. It checks whether entries are in the child list when adding which is O(n^2) in the number of items from the registry. It would be O(n) to use an IdentityHashMap as a set, but probably not worth it since the number of entries is likely to be small. Reversing the order of adding to be registry then boundServices may be better (though then n^2 in the number of boundServices matches)

@wildfly-ci
Copy link

Triggering build using a merge of 6685a8c on branch master:
http://lightning.mw.lab.eng.bos.redhat.com/jenkins/job/as7-param-pull/

@wildfly-ci
Copy link

Build 3839 is now running using a merge of 6685a8c on branch master:
http://lightning.mw.lab.eng.bos.redhat.com/jenkins/job/as7-param-pull/3839

@wildfly-ci
Copy link

Build 3839 outcome was FAILURE using a merge of 6685a8c on branch master:
http://lightning.mw.lab.eng.bos.redhat.com/jenkins/job/as7-param-pull/3839

@doctau
Copy link
Author

doctau commented Sep 6, 2012

Is that just an intermittent test failure?

java.util.concurrent.TimeoutException
at java.util.concurrent.FutureTask$Sync.innerGet(FutureTask.java:228)
at java.util.concurrent.FutureTask.get(FutureTask.java:91)
at org.jboss.as.test.integration.common.HttpRequest.execute(HttpRequest.java:49)
at org.jboss.as.test.integration.common.HttpRequest.get(HttpRequest.java:79)
at org.jboss.as.test.integration.jaxrs.async.JaxrsAsyncTestCase.performCall(JaxrsAsyncTestCase.java:66)
at org.jboss.as.test.integration.jaxrs.async.JaxrsAsyncTestCase.testJaxRsWithNoApplication(JaxrsAsyncTestCase.java:71)

@kabir
Copy link
Contributor

kabir commented Sep 6, 2012

Retest this please!

@wildfly-ci
Copy link

Triggering build using a merge of 6685a8c on branch master:
http://lightning.mw.lab.eng.bos.redhat.com/jenkins/job/as7-param-pull/

@wildfly-ci
Copy link

Build 3849 is now running using a merge of 6685a8c on branch master:
http://lightning.mw.lab.eng.bos.redhat.com/jenkins/job/as7-param-pull/3849

@wildfly-ci
Copy link

Build 3849 outcome was SUCCESS using a merge of 6685a8c on branch master:
http://lightning.mw.lab.eng.bos.redhat.com/jenkins/job/as7-param-pull/3849

@n1hility
Copy link
Member

n1hility commented Sep 7, 2012

We need to find another way to address this problem. This solution doesn't work for all cases and as you mention it's going to be terribly slow.

@stuartwdouglas
Copy link
Contributor

I have addressed this a different way in #3026.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants